-
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] - Starter Page Templates - Picker and Preview design updates #20959
Changes from 2 commits
abdeae5
bbed05c
85f1404
fab02d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,10 +4,17 @@ | |
import { isUnmodifiedDefaultBlock } from '@wordpress/blocks'; | ||
import { useSelect } from '@wordpress/data'; | ||
|
||
const __experimentalUsePageTemplatePickerVisible = () => { | ||
export const __experimentalUsePageTemplatePickerAvailable = () => { | ||
return useSelect( ( select ) => { | ||
const { getCurrentPostType } = select( 'core/editor' ); | ||
return getCurrentPostType() === 'page'; | ||
}, [] ); | ||
}; | ||
|
||
export const __experimentalUsePageTemplatePickerVisible = () => { | ||
const isTemplatePickerAvailable = __experimentalUsePageTemplatePickerAvailable(); | ||
|
||
return useSelect( ( select ) => { | ||
const { getBlockOrder, getBlock } = select( 'core/block-editor' ); | ||
|
||
const blocks = getBlockOrder(); | ||
|
@@ -16,10 +23,7 @@ const __experimentalUsePageTemplatePickerVisible = () => { | |
const isOnlyUnmodifiedDefault = | ||
blocks.length === 1 && isUnmodifiedDefaultBlock( firstBlock ); | ||
const isEmptyContent = isEmptyBlockList || isOnlyUnmodifiedDefault; | ||
const isPage = getCurrentPostType() === 'page'; | ||
|
||
return isEmptyContent && isPage; | ||
return isEmptyContent && isTemplatePickerAvailable; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm always down for performance improvements 🙌, I'll definitely check it out, thanks for the review koke! |
||
}, [] ); | ||
}; | ||
|
||
export default __experimentalUsePageTemplatePickerVisible; |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { createHigherOrderComponent } from '@wordpress/compose'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { | ||
__experimentalUsePageTemplatePickerVisible, | ||
__experimentalUsePageTemplatePickerAvailable, | ||
} from './use-page-template-picker'; | ||
|
||
const __experimentalWithPageTemplatePicker = createHigherOrderComponent( | ||
( WrappedComponent ) => ( props ) => { | ||
const isTemplatePickerVisible = __experimentalUsePageTemplatePickerVisible(); | ||
const isTemplatePickerAvailable = __experimentalUsePageTemplatePickerAvailable(); | ||
|
||
return ( | ||
<WrappedComponent | ||
{ ...props } | ||
isTemplatePickerVisible={ isTemplatePickerVisible } | ||
isTemplatePickerAvailable={ isTemplatePickerAvailable } | ||
/> | ||
); | ||
}, | ||
'__experimentalWithPageTemplatePicker' | ||
); | ||
|
||
export default __experimentalWithPageTemplatePicker; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,5 +56,5 @@ | |
} | ||
|
||
.separatorDark { | ||
background-color: $gray-70; | ||
background-color: #2d2d2d; | ||
} |
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 wrapper will solve the issue with the transparency of the buttons