[RNMobile] - Starter Page Templates - Picker and Preview design updates #20959
Conversation
|
Size Change: 0 B Total Size: 856 kB ℹ️ View Unchanged
|
geriux
force-pushed
the
rnmobile/spt-design-improvements
branch
from
f31e4b0
to
abdeae5
Compare
| @@ -143,8 +144,10 @@ class Layout extends Component { | |||
| parentHeight={ this.state.rootViewHeight } | |||
| style={ toolbarKeyboardAvoidingViewStyle } | |||
| > | |||
| { showPageTemplatePicker && ( | |||
| <__experimentalPageTemplatePicker /> | |||
| { isPage && ( | |||
There was a problem hiding this comment.
Before, the component was being unmounted after the user has at least one block. To be able to have a fade-out animation, the component can't be unmounted. So now the picker will be rendered if its a page only (since we don't want to import everything for a standard post)
Within the component, it will handle if it should be rendered or not by passing showPageTemplatePicker as a prop.
| @@ -6,12 +6,21 @@ | |||
| padding: 0 12px; | |||
| } | |||
|
|
|||
| .button { | |||
| background-color: rgba($black, 0.04); | |||
| .buttonWrapper { | |||
There was a problem hiding this comment.
This wrapper will solve the issue with the transparency of the buttons
geriux
changed the title
geriux
added
the
Mobile App - i.e. Android or iOS
geriux
requested review from
koke and
mkevins
and removed request for
youknowriad and
ellatrix
|
Looking at the animations, it feels odd that the buttons are still there when the preview is dismissed. I think the most useful case for the animation is when you don't want to use a template, and they disappear when you stop writing. When this happens, there's a ~500ms delay between the text showing on screen and the fade out animation starting.
Edit: I just saw the delay was intentional, but still not sure why it's there |
To be honest I thought that was what we got requested from @iamthomasbishop but let's double check 😄 Thomas in what case you'd like to see the fade out animation? It is true that feels odd to see the buttons after the modal is dismissed (due the delay I added to make this happen). If it's just when a user is typing I'll remove the delay so it starts animating right away =) |
Ahh that does look odd when the modal transitions out. I was mainly referring to when you start adding content (text or use the inserter). The 500ms delay looks nice in that case. In terms of the logic relevant to the modal:
|
I imagine this will be barely visible, as the modal animation seems faster than the fade out.
If we fade them out when presenting the modal, then you can't fade them out when closing it 😁 |
| const { getEditorMode } = select( 'core/edit-post' ); | ||
|
|
||
| return { | ||
| isPage: getCurrentPostType(), |
There was a problem hiding this comment.
I think this should be getCurrentPostType() === 'page'.
I think I'd still encapsulate that logic in the HOC, maybe splitting the PageTemplatePickerVisible HOC/hook into two: "available" (is a page) and "visible" (has no content).
|
Other than the comment I left, the code looks fine. I'm waiting to clarify what the animation should be before final testing |
True 👍
Definitely, I just wanted to confirm what should happen in each case because in the original examples they were re-appearing momentarily 😄 Is there an updated test build somewhere that I can try out? |
|
I have tested this on Android via |
Hey @iamthomasbishop 👋 I've just made one with some changes, let me know how the animation looks. Thanks!!
|
Animations look smooth! 👍 |
|
|
||
| return isEmptyContent && isPage; | ||
| return isEmptyContent && isTemplatePickerAvailable; |
There was a problem hiding this comment.
I'm not sure if I'm overthinking this, but I imagine the majority of times this runs it will return false. I'd suggest refactoring a bit to return early and avoid any extra calculations:
if ( ! isTemplatePickerAvailable ) {
return false;
}
//...
if ( isEmptyBlockList ) {
return false;
}
// ...It'd be interesting to check if this impacts performance at all, or I'm just worried about nothing 😅
There was a problem hiding this comment.
I'm always down for performance improvements 🙌, I'll definitely check it out, thanks for the review koke!
Gutenberg Mobile PR-> wordpress-mobile/gutenberg-mobile#2027Description
This PR addresses some design feedback for the Starter Page Templates Picker and Preview Modal:
It also solves this transparency issue.
Screenshots
How has this been tested?
Test 1
Test 2 [iOS]
Types of changes
Design improvements
Checklist: