-
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
Fix custom patterns console error #51947
Conversation
Size Change: +11 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
Flaky tests detected in 62b0996. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5397758763
|
if ( syncStatus === 'unsynced' ) { | ||
return reusableBlockInserterItems; | ||
} |
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.
In hindsight, there should have been some previous feedback against the addition of the sync_status
argument to this selector.
With this change, it feels like a good idea to go ahead and remove it. The unsynced
patterns could be returned by a new private selector.
edit: I tried something out in #51955, which is now merged into 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.
Code looks good to me! Only a small suggestion but that's not blocking 👍
|
||
// Hide block pattern preview on unmount. | ||
useEffect( () => () => onHover( null ), [] ); | ||
useEffect( () => () => onHover( null ), [ onHover ] ); |
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 might be a bit dangerous since we didn't require onHover
to be a stable function instance. Which means someone might accidentally pass in an unmemoized function and trigger this effect every render.
We might want to:
- Use a ref to create a (semi-)stable reference to the function
- Ignore the eslint warning and add comment above explaining why.
3101fe1
to
d7cad51
Compare
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 didn't do much testing but this looks good to me.
@@ -2082,7 +2083,7 @@ export const getInserterItems = createSelector( | |||
{ core: [], noncore: [] } | |||
); | |||
const sortedBlockTypes = [ ...coreItems, ...nonCoreItems ]; | |||
return [ ...sortedBlockTypes, ...reusableBlockInserterItems ]; | |||
return [ ...sortedBlockTypes, ...syncedPatternInserterItems ]; |
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.
Does it still make sense to keep the "reusable blocks" in this selector, now that they're supposed to be treated as a pattern instead (and are part of the other selector)?
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.
Currently, the synced blocks still appear in the quick inserter list so are needed here. We can pull in from another selector of course, but there are some inconsistencies with when/where the different patterns appear which need to be sorted out, eg. so I suggest we leave this as is for this PR and circle back once we have finalised exactly how/when the different sync status types will be listed.
d7cad51
to
602e389
Compare
* Add custom patterns to pattern explorer * show custom patterns in the patterns explorer dialog * remove changes from 51877 * Fix up use of async lists * remove a bit of code duplication by adding a new hook * add 51877 fix back to make testing easier
602e389
to
62b0996
Compare
* Fix error with react list key with new custom patterns list in inserter * Update placeholder key * Add comment to explain the different keys * Patterns: Fix missing custom patterns in patterns explorer (#51889) * Add custom patterns to pattern explorer * show custom patterns in the patterns explorer dialog * remove changes from 51877 * Fix up use of async lists * remove a bit of code duplication by adding a new hook * add 51877 fix back to make testing easier * Just assign the key value in one place * Refactor the custom patterns to use the usePatternsState hook * Fix use of async list * Translate strings and remove unneeded fields from pattern object * Try integrating unsynced patterns directly into pattern selectors (#51955) * Include reusable blocks with an undefined sync status in inserter items * Update docs * Remove change to hover dependencies --------- Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
I just cherry-picked this PR to the update/wp6-3-beta3 branch to get it included in the next release: db15884 |
* Footnotes: inscrease selector specificity for anchor (#52179) * Patterns: Include template parts for custom areas in Uncategorized category (#52159) * Fix custom patterns console error (#51947) * Fix error with react list key with new custom patterns list in inserter * Update placeholder key * Add comment to explain the different keys * Patterns: Fix missing custom patterns in patterns explorer (#51889) * Add custom patterns to pattern explorer * show custom patterns in the patterns explorer dialog * remove changes from 51877 * Fix up use of async lists * remove a bit of code duplication by adding a new hook * add 51877 fix back to make testing easier * Just assign the key value in one place * Refactor the custom patterns to use the usePatternsState hook * Fix use of async list * Translate strings and remove unneeded fields from pattern object * Try integrating unsynced patterns directly into pattern selectors (#51955) * Include reusable blocks with an undefined sync status in inserter items * Update docs * Remove change to hover dependencies --------- Co-authored-by: Daniel Richards <daniel.richards@automattic.com> * i18n: Add context to the word "Filters" (#52198) * Update home template icon (#52075) * Centralise all permissions lookup in Link UI and enable (#52166) * BlockRemovalWarningModal: Fix incorrect '_n' usage (#52164) * Fix fetching Nav fallback ID flushing Navigation entity cache (#52069) * Only flush the `getEntityRecords` cache if the fallback isn’t already in state * Save the edited entity record to a const and then invert it to determine whether we should invalidate the recordds --------- Co-authored-by: scruffian <ben@scruffian.com> * Block Editor: Unify texts for Create pattern modal (#52151) * Fix history back after entering edit mode from Patterns (#52112) * Add template part icons to the library grid items (#51963) * Patterns: Fix sidebar tab label (#51953) * Patterns: Fix setting of sync status for fully synced patterns (#51952) * Library: Reinstate manage all template parts page (#51961) * Command Palette: fix incorrect path and snackbar message when template part is deleted (#52034) * Command Center: Fix incorrect navigation when deleting template part * removeTemplate: consider title type * Drop-indicator: remove white border. (#52122) * Make Navigation fallback selector private (#51413) * Move selector to become private * adds basic lock functionality * remove useless lock-unlock file * map private selectors to resolvers * Unlock the other usage * only create one fallback per session * Fix core-data duplicate private opt-in * Data: bind resolvers to selectors individually, support private selectors --------- Co-authored-by: Andrei Draganescu <andrei.draganescu@automattic.com> Co-authored-by: scruffian <ben@scruffian.com> Co-authored-by: Jarda Snajdr <jsnajdr@gmail.com> * Focus Mode: Use the symbol icon if a pattern is being edited (#52031) * Footnotes: register meta field for pages (#52024) * Fix unintentional toggling on of distraction free (#52090) * replace toggle with set preference - because I don't read code properly it seems * remove notification * Revert "Updating social link attributes (#51997)" (#52019) This reverts commit c711e2a. * Update home template name (#52048) * Removes unused call (#51988) * Remove ability for user to toggle sync status after pattern creation (#51998) * Fix disable DFM when opening styles command (#52165) * Update custom patterns label to 'My patterns' (#51949) * rename custom patterns to my patterns * Add my patterns label to inserter and show at the top --------- Co-authored-by: Daniel Richards <daniel.richards@automattic.com> * Library: Add sync status to pattern details screen (#51954) * Patterns: Rename Library to Patterns (#52102) * [Library] Add lock icon for theme patterns (#51990) * Add lock icon for theme patterns * Change to class names * Add aria-description * Change wording * Patterns: Use "detached" copy consistently (#51993) * Editor initrial appender: Zero out margins in constrained layouts. (#52026) * Update pattern creation modal in library (#51946) * Fix missing snackbars in Library (#52021) * Make the entire preview clickable in order to enter "edit" mode in focus mode (#51973) * Page Content Focus: Add welcome guides (#52014) * Page Content Focus: Add welcome guides * Don't show when editor guide is active * Just use regular accent/theme color in all guides * slight copy change page guide * Update components changelog * Disable new guides in E2E tests * Use s.w.org videos --------- Co-authored-by: Saxon Fletcher <saxonafletcher@gmail.com> --------- Co-authored-by: Ella <4710635+ellatrix@users.noreply.github.com> Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Co-authored-by: Glen Davies <glendaviesnz@users.noreply.github.com> Co-authored-by: Daniel Richards <daniel.richards@automattic.com> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Co-authored-by: James Koster <james@jameskoster.co.uk> Co-authored-by: Dave Smith <getdavemail@gmail.com> Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com> Co-authored-by: scruffian <ben@scruffian.com> Co-authored-by: Kai Hao <kevin830726@gmail.com> Co-authored-by: Carolina Nymark <myazalea@hotmail.com> Co-authored-by: Joen A <1204802+jasmussen@users.noreply.github.com> Co-authored-by: Andrei Draganescu <andrei.draganescu@automattic.com> Co-authored-by: Jarda Snajdr <jsnajdr@gmail.com> Co-authored-by: Andrei Draganescu <me@andreidraganescu.info> Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com> Co-authored-by: Saxon Fletcher <saxonafletcher@gmail.com> Co-authored-by: Rich Tabor <hi@richtabor.com> Co-authored-by: Robert Anderson <robert@noisysocks.com>
* Fix error with react list key with new custom patterns list in inserter * Update placeholder key * Add comment to explain the different keys * Patterns: Fix missing custom patterns in patterns explorer (WordPress#51889) * Add custom patterns to pattern explorer * show custom patterns in the patterns explorer dialog * remove changes from 51877 * Fix up use of async lists * remove a bit of code duplication by adding a new hook * add 51877 fix back to make testing easier * Just assign the key value in one place * Refactor the custom patterns to use the usePatternsState hook * Fix use of async list * Translate strings and remove unneeded fields from pattern object * Try integrating unsynced patterns directly into pattern selectors (WordPress#51955) * Include reusable blocks with an undefined sync status in inserter items * Update docs * Remove change to hover dependencies --------- Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
What?
Fixes console error when browsing
Custom patterns
in the inserter patterns panel.Why?
An alternative fix to #51877 for the following issues.
Fixes: #51873 and #51872
How?
If the pattern is a
core/block
sets the patternname
toid
.Includes a larger refactor compared to #51877 based on feedback there. Rather than trying to combined patterns and reusable blocks in components, instead moves to doing so in any pattern related selectors.
Testing Instructions
For #51873
For #51872
Screenshots or screencast
Before:
10d517cb75e3b47b76755c52c24bc53c.mp4
c22f1fcfc097fa80150a073597e14f6f.mp4
After:
patterns-bug.mp4
duplicate.mp4