-
-
Notifications
You must be signed in to change notification settings - Fork 490
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
feat: Mobile-optimized formatting toolbar #1284
Conversation
# Conflicts: # package-lock.json
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@areknawo is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
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.
Nice! Turned out more simple than expected and great job researching everything re. visualviewport. No questions about the code atm.
- Can you add an "example" (/examples) that uses the experimental controller?
- Create a PR description
After that, please assign to @matthewlipski so he can do final review / merge
...ges/react/src/components/FormattingToolbar/ExperimentalMobileFormattingToolbarController.tsx
Show resolved
Hide resolved
examples/03-ui-components/14-experimental-mobile-formatting-toolbar/App.tsx
Outdated
Show resolved
Hide resolved
@matthewlipski could you review this PR? |
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.
The formatting toolbar fixes for mobile look good!
The ExperimentalMobileFormattingToolbarController
looks like a good first step, however I noticed a few UX issues on mobile that are pretty significant. Not sure if those something that needs to be cleared before merging since it is an experimental feature @YousefED.
- The toolbar opens after highlighting some text, but then stays open when the selection is collapsed. I think the behaviour should be the same as in the regular formatting toolbar positioner.
- Tapping an element in the formatting toolbar looks like it causes the editor to lose focus, as the keyboard also disappears.
- The dropdown menus in the formatting toolbar aren't visible when open.
|
||
This example shows how to use the experimental mobile formatting toolbar, which uses [Visual Viewport API](https://developer.mozilla.org/en-US/docs/Web/API/Visual_Viewport_API) to position the toolbar right above the virtual keyboard on mobile devices. | ||
|
||
Controller is currently marked **experimental** due to the flickering issue with positioning (caused by delays of the Visual Viewport API) |
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.
When is the flickering visible? I don't think I saw it when playing around in the preview
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.
Did you test on mobile? When you scroll / down a large document the updating of the position is a bit delayed
Good UX feedback @matthewlipski. The experiment is mainly about the positioning. If that, and the code looks good, I'm ok with merging it for now. We can decide to iterate on it later. Another (valid) option is to separate it into a draft PR |
This PR optimizes the formatting toolbar for mobile, limiting its width and enabling position shifting and overflow when necessary.
Additionally, it includes an
ExperimentalMobileFormattingToolbarController
for future reference and possible development. This controller aims to use the Visual Viewport API to position the toolbar right above the virtual keyboard on mobile devices. It's currently marked experimental due to the flickering issue with positioning (caused by delays of the Visual Viewport API)