-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[RNMobile] - Layout Picker - Keyboard visibility improvements #21463
Conversation
@@ -8,7 +8,7 @@ import { useDispatch, useSelect } from '@wordpress/data'; | |||
* External dependencies | |||
*/ | |||
import { logUserEvent, userEvents } from 'react-native-gutenberg-bridge'; | |||
import { Animated } from 'react-native'; | |||
import { Animated, Dimensions, Keyboard } from 'react-native'; |
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.
Note: Didn't use useWindowDimensions because there's a bug that will be fixed for 0.62
Size Change: -240 B (0%) Total Size: 889 kB
ℹ️ View Unchanged
|
Checked on a few simulators and it looks good there.
I also tested on an iPhone 11 (Physical Device). When testing on my physical device I noticed a bit of a flash of the toolbar on the rotate it's really quick. It's hard to see in a Gif so I uploaded a video you can see the flash at 0:05 and again at 0:09 I also noticed a few things in portrait mode. I know this issue is focusing on landscape mode but wanted to get your thoughts. On an iPhone 5s (Simulator) and iPhone SE (Simulator), the controls still cover the editor a little bit. However, this might be acceptable to not completely disable the feature for users.
Although the overlap is a bit more pronounced on smaller Android devices like this Nexus One What are your thoughts on these @geriux? |
Hey @chipsnyder ! Thank you for testing and reviewing!
Nice catch! I did some changes so it would fade out instead of flash like in the video, here's how is looking now: Simulator: Real device: Note that we are testing in DEV mode. When this ships, all performance improvements and animations optimizations will be turned on so it will run even smoother.
For these cases the issue is different because there is more room to scroll so I think we are ok: Still, if you think it doesn't look right we can talk about it and find an improvement for 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.
I did some changes so it would fade out instead of flash like in the video, here's how is looking now:
This animation looks a lot better now! 🎉
Note that we are testing in DEV mode. When this ships, all performance improvements and animations optimizations will be turned on so it will run even smoother.
That's a great point I wasn't thinking about that.
TIL Common sources of performance problems. Running in development mode (dev=true)
For these cases the issue is different because there is more room to scroll so I think we are ok:
That's another good point!
LGTM!
Gutenberg Mobile PR
-> wordpress-mobile/gutenberg-mobile#2123Description
While testing the picker functionality it was found that on some smaller devices the picker would cover part of the editor.
It was agreed to tweak the visibility of the picker depending on:
From wordpress-mobile/gutenberg-mobile#2088 (comment)
Screenshots
iPhone 6S
iPhone 11
iPad
Android
How has this been tested?
On a mobile device
On a tablet
Note: While on portrait the only time the picker should be hidden is when there is content in the editor. E.g the first Paragraph block with some text.
Types of changes
Improvement
Checklist: