[Library] Add lock icon for theme patterns #51990
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevin940726
added
[Feature] Patterns
92 tasks
|
Size Change: +353 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for the quick update on this @kevin940726
It tests well for me. The only catch worth noting is that we might need to time merging this with #51949 so that the editable user-created patterns don't also get the lock icon beside the category.
It would also be nice if we can clean up the inline style use as well.
packages/edit-site/src/components/sidebar-navigation-screen-library/index.js
Outdated
Show resolved
Hide resolved
Thanks for flagging that! I'll convert this to draft then so we don't accidentally merge it. |
|
Nice, this is working well. Here a GIF showing the tooltips:
A quick note, every pattern inside "Custom patterns" are actually editable, so those should not have the lock: 
I'd love a sanity check by @jameskoster but otherwise, this feels like a good step forward. |
Yeah, I'm waiting for #51949 to be merged so that I can rebase the logic on top of it. 👍 |
Seems fine to me, once the Custom Pattern category is fixed. We'll need to remove the lock icon on the menu items once custom patterns can be categorised, or theme patterns edited. For that reason we might consider only displaying it on the patterns in the grid. It depends on how soon either of those features land. |
talldan
reviewed
Jun 28, 2023
| <Tooltip | ||
| position="top center" | ||
| text={ __( | ||
| 'Theme patterns cannot be edited here' |
There was a problem hiding this comment.
I guess the tooltip can't be read by a screenreader. We may want to think about options for that.
We probably don't want to make each item too verbose, but a little hint could be good. I think there is already an aria-description, so that's an option.
There was a problem hiding this comment.
Yeah the whole header is not accessible. I added aria-description in 8c2d390 but I agree it could be a bit verbose. I wonder if we should move that part to the parent section instead? Not sure about the accessibility implication here 🤔 .
|
I'm not quite happy with the wording "This pattern can not be edited here". |
That's the tricky thing. The source of the pattern cannot be edited, as it's a PHP file on the server. Soon enough in the plugin, but not in 6.3 you will be able to edit it, just like you can edit templates (with a "delete customizations"). So in the mean time, the wording is tricky. Because given it's a pattern, you can insert it anywhere and edit it after the fact in a post or a page. So it's just not the source you can edit. |
kevin940726
force-pushed
the
add/library-theme-pattern-icon
branch
from
4c9e280
to
308ac6f
Compare
|
I just rebased it now that #51949 is merged. This is what it currently looks like when there's user patterns:
|
|
Flaky tests detected in 30804a8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5420167365
|
Got it. I'm just a little bothered by the "here" because that implies that you can edit it somewhere else. However, editing theme PHP files is for the normal user de facto like not editable - we also could just say that by leaving out the here: "Theme patterns cannot be edited". |
That can work. |
|
2125221 changed the wording to |
kevin940726
force-pushed
the
add/library-theme-pattern-icon
branch
from
2125221
to
30804a8
Compare
aaronrobertshaw
approved these changes
Jun 30, 2023
There was a problem hiding this comment.
Thank you for continuing to iterate on this one @kevin940726 👍
It tests well for me and I think this is in a good place.
🚢
glendaviesnz
added
the
Backport to WP 6.7 Beta/RC
tellthemachines
pushed a commit
that referenced
this pull request
Jul 3, 2023
* Add lock icon for theme patterns * Change to class names * Add aria-description * Change wording
|
I just cherry-picked this PR to the update/wp6-3-beta3 branch to get it included in the next release: e29a7c2 |
tellthemachines
removed
the
Backport to WP 6.7 Beta/RC
tellthemachines
added a commit
that referenced
this pull request
Jul 3, 2023
* 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>
I think there's a bit of confusion about
Instead, this PR makes 
It should hold the ID reference of the hidden div that holds the description. This should be fixed. Will create a new issue to report also more things that need some love. Cc/ @kevin940726 |
|
@afercia Oops! I passed in the wrong values 🤦. I'm guilty of not testing the last change 😞 . I'll fix it tomorrow morning. Thanks for letting me know! |
41 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What and why?
See #51948 (comment).
How?
Just some flexbox thing and add the icon.
I noticed that some part of the library isn't accessible. It's not related to this PR though.
Testing Instructions
Testing Instructions for Keyboard
TBD
Screenshots or screencast