Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use simpleMDE editor on mobile devices for 1.13 #14029

Merged
merged 4 commits into from
Dec 17, 2020

Conversation

mrsdizzie
Copy link
Member

simpleMDE doesn't work properly on mobile devices -- We've replaced it with the slightly more working easyMDE in 1.14 but since that change can't be backported to 1.13 we will just disable the editor on mobile here.

Quote reply seemed to assume it had been initialized so I just set it back to text area after to avoid breaking/changing that code on a backport.

simpleMDE doesn't work properly on mobile devices -- We've replaced it with the slightly more working easyMDE in 1.14 but since that change can't be backported to 1.13 we will just disable the editor on mobile here.
@mrsdizzie mrsdizzie added the pr/wip This PR is not ready for review label Dec 16, 2020
web_src/js/index.js Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 16, 2020
Co-authored-by: silverwind <me@silverwind.io>
@6543 6543 added this to the 1.13.1 milestone Dec 16, 2020
@silverwind
Copy link
Member

silverwind commented Dec 16, 2020

  1. Put isMobile into a function in utils.js, making it accessible to other files as well.
  2. Not sure how much work it would be but instead of reverting to textarea we should ideally never convert it in first place.

@mrsdizzie
Copy link
Member Author

mrsdizzie commented Dec 16, 2020

Its converted back to text because the quote reply seems to rely on it being initialized and doesn't work otherwise. since it is just a 'backport'/quick fix I didn't want to mess with that more then needed

This pr is directly to 1.13 since people didn't want to change it for 1.14 -- could make better changes there maybe

@mrsdizzie
Copy link
Member Author

mrsdizzie commented Dec 16, 2020

@silverwind made it a function in utils.js

disabled for code-review and code-review replies also. listed as WIP right now because wiki plain text editing in general is currently broken on 1.13 so unclear how/if this PR would address that.

@mrsdizzie mrsdizzie changed the title Don't use simpleMDE editor on mobile devices Don't use simpleMDE editor on mobile devices for 1.13 Dec 17, 2020
@mrsdizzie mrsdizzie removed the pr/wip This PR is not ready for review label Dec 17, 2020
@mrsdizzie
Copy link
Member Author

OK also fixed bug for switching to plaint text mode on wiki that is present for all 1.13 use (desktop and mobile). Was separately fixed in master from this commit:

374ff60#diff-b50d16bb0041e8bc2ab51127d1231e8ac8bf982ed46e7aaa198da454ef124f95L56-L59

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 17, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 17, 2020
@6543
Copy link
Member

6543 commented Dec 17, 2020

🚀

@6543 6543 merged commit 4f296f7 into go-gitea:release/v1.13 Dec 17, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
@lunny
Copy link
Member

lunny commented Jan 26, 2021

This resulted in #14196. Should we revert this PR?

@zeripath
Copy link
Contributor

I'd argue not - we should fix the IsMobile function instead.

export function isMobile() {
  return /Mobi/.test(navigator.userAgent);
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants