Page List: Update the icon to demonstrate that the pages are automatically updated #46438
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
scruffian
requested review from
draganescu,
getdave,
SaxonF,
jasmussen,
MaggieCabrera and
ajlende
|
Size Change: +657 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
|
We shouldn't use the block icon for this, IMO. (Also "update" is still a dashicon.) |
|
To echo @mtias the "update" icon only makes sense when we display the page list inside the navigation tree. But when searching in the inserter users should see a "pages" icon. |
|
I updated this so that we pass a context to the icon function, which allows us to display different icons in different contexts. This could also be useful for other blocks. |
|
Nice. I wonder if we should use this icon for it?
While we created that icon for reusable blocks, it's implication is that it's a "lightweight synced block", in compareson with the solid version, which is a template part:
I took a stab at a bunch of looks to share DNA with that, if we were to create a new one. None of which I like, but sharing for fun:
In the end, I landed here: If @SaxonF has an opinion, I'd refer to him. But otherwise, here's the SVG: What should we call it? Not "synced-block" as above, because reusable blocks might be called that. Perhaps just "sync"? |
|
Or should this just be a replacement for "update"? |
Oh right, Update exists, it's just using the legacy dashicons vectors. Yes, definitely update that one. |
|
I updated the SVG: |
getdave
approved these changes
Dec 14, 2022
There was a problem hiding this comment.
This works as described. In Nav block offcanvas list view only it appears as synced icon.
I noticed in the main List View it appears as normal. Is that intentional?
The inserter appears as the default icon which is 👍
Last thing (personal preference alert) I don't think the icon looks that great. It's very small and hard to distinguish.
Also from a technical standpoint, we should be thinking of a way to communicate the "Synced" status in a non-visual manner for assistive tech.
dmsnell
pushed a commit
that referenced
this pull request
Dec 15, 2022
…cally updated (#46438) * Page List: Update the icon to demonstrate that the pages are automatically updated * Allow a context to be passed to the block icon
23 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?
This updates the Page List icon to make it clear that the list of pages are synced with the settings in wp-admin.
Why?
This was originally proposed in #42257.
How?
This simply changes the icon. We need a new icon as this one is ugly. cc @jasmussen
Testing Instructions
3. Check that the icon in the inserter still looks the same.