-
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
[RNMobile] Editor Onboarding: "The Basics" help section - Details Part 1 #33790
Conversation
Size Change: -453 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
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.
This looks good, @AmandaRiu! Thank you for putting this work together. I verified the feature works as expected on an iPhone SE and Samsung Galaxy S20.
I agree that a fullscreen bottom sheet likely makes a lot of sense. Currently, the help section title and back button scrolls out of view when viewing longer content. Losing visibility of the back button in particular feels odd and disorienting. Hopefully we can improve this aspect in future work.
I left a few comments that are not blockers. We could always address them in future PRs if desired.
packages/editor/src/components/editor-help/add-blocks.native.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/editor-help/help-detail-navigation-screen.native.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/editor-help/view-sections.native.js
Outdated
Show resolved
Hide resolved
Loading local static assets from a different package within a monorepo appears broken in Metro for Android. The below fixes paths to packages within this project to include the necessary `/assets/..` that Metro's server expects to traverse to the correct directory. - https://git.io/JBV4e - https://git.io/JBFon
Gutenberg-Mobile PR: wordpress-mobile/gutenberg-mobile#3777
Description
This PR is the second in a series of PRs to add the Editor Onboarding basic help section defined in this ticket: #30626 and covers the following:
Note that this PR only covers a baseline for this area of the feature and will continue to be iterated on by another developer. I'm opening this PR now to ensure the work done so far so it can neatly be handed off to the next developer as I'll be moving projects.
Special notes:
BottomSheet.SubSheet
for the details pages, but for some reason the images would flicker uncontrollably while scrolling on iOS. I attempted to useuseMemo
to prevent this, but couldn't get it to work. For that reason I went back to my original custom implementation ofBottomSheet.NavigationScreen
and useuseMemo
inview-section.native.js
when serving up the image assets to avoid rerendering images while scrolling. This works nicely. Below are recordings of the iOS differences.The "before" is using
BottomSheet.SubSheet
:subsheet.mp4
After is using the custom
BottomSheet.NavigationSheet
:old-sheet.mp4
How has this been tested?
Screenshots
Screen-Recording-2021-07-31-at-5.13.57-AM.mp4
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).