-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Mobile: Fix toolbar position #6307
Conversation
Previously we didn't have this option as it was broken, if this now isn't as broken as in #4079 totally ok with it being back. That was a force to get around the visual issues in past. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the code point of view changes here look good. I tested and it has the correct functionality, it forces the toolbar to be on the top on mobile as expected.
Given that now it is possible to show toolbar at the top on mobile I wonder if now we should not just let mobile users choose their preference like desktop users can do.
Thanks for working on this, and thanks for the context, Tammie. It looks like the problem that the switch was intended to solve hasn't been fixed: You still have to scroll to the top. But at least now you can select text and use the bold/italic, without covering them with the cut/copy/paste iOS selection thing. I'm gong to take a look at fixing the toolbar so it's always visible — it should be sticky. Going to try and see if I can push a fix to this branch. |
So as it turns out, we might need a "clever trick" for this. The problem, which I hoped moving the toolbar to the top would solve, was that the popup cut/copy/paste dialog in iOS covered the block toolbar. The problem is as soon as the soft keyboard shows up on iOS, well essentially position: sticky and position: absolute stop working. Here's what Notion does: It's basically a popup toolbar that shows up just on setting the cursor. Not ideal. I'm attempting some of the hacks suggested in https://gist.github.com/avesus/957889b4941239490c6c441adbe32398. As a reminder, this is what master looks like right now: Note how words on line 1 are blocked by the ios popup toolbar. |
So, after investigating this for a bit, I believe there are only a few options on our table. This is keeping in mind that it feels as if mobile safari is simply not designed to allow writing experiences — Google Docs, Medium both block writing in mobile browsers and require you to use the app. The first option is to keep the toolbar fixed to the top, and... do nothing. Simply accept that you have to scroll aaaaall the way up there if you need to make text bold, or create links. Not ideal. This is what WordPress.com does now, this is what the classic editor does now: The other option is to enter serious hack terrritory. It involves us blocking all touchstart and touchend calls on iOS, so you can't scroll the The third option is to look at what Notion.so has done, and either do the same — create a "popup toolbar" that sits below the cursor. Or, we can rearrange the block toolbar to be below the block, so you can't cover it with the iOS cut/copy/paste bubble. Something like this: It's not ideal, but it feels like the solution that will work the best for us. |
This is very much an experiment, intended to help solve problems discovered in #6307 (comment). Basically, on iOS you won't have access to the block toolbar until you scroll way up to the top. And if we dock the toolbar to the top of the block, it will be covered by cut/copy/paste. This PR moves the toolbar below the block instead, mitigating that.
Took a stab at a different solution in #6347. |
Should we close this one as it seems like we are going with #6347? |
This is very much an experiment, intended to help solve problems discovered in #6307 (comment). Basically, on iOS you won't have access to the block toolbar until you scroll way up to the top. And if we dock the toolbar to the top of the block, it will be covered by cut/copy/paste. This PR moves the toolbar below the block instead, mitigating that.
closes #6236
The mobile toolbar should always be shown as fixed.