Try a single icon for title blocks #40596
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
jameskoster
added
Needs Design Feedback
|
Size Change: -763 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
|
Nice PR! I think it's good to pull the breaks on the proliferation of title icons like this — there's an argument for every block needing a unique icon, to be sure, but these title variants haven't quite been clear enough on their own. I'd use the T with two lines below, though, rather than the big T. |
|
Title icon updated:
|
jasmussen
reviewed
Apr 27, 2022
There was a problem hiding this comment.
I'd be happy to try this. Not because the T + 2 lines icon is necessarily the best, but because the variants of it (A/C/etc) are not good and it'd be good to curtail that design.
Two things, though:
- You're changing the Title icon. I don't know if the T is used anywhere, I don't think it is, but that means it needs a changelog update. https://github.com/WordPress/gutenberg/blob/trunk/packages/icons/CHANGELOG.md
- Since we are consolidating, should we deprecate/delete all the variant icons that get replaced with this? Or is that something we should postpone until later?
|
I'd be anxious about deleting any icons in the set since we don't know for sure whether they're being used by third parties. |
Indeed. But we've done this before, and I think it important we do that for mistakes made lest they build up over time. In the case of NPM, the change will be noted as breaking in the changelog, and you can then choose not to update, or address the breaking change while doing so. In this case, since the PR is in the "try" category, I'd be happy to try this one without any deprecations, but my main concern there is: if we end up liking this change, eventually we'll want to deprecate the icons and remove them from the library. What do you think? CC: @javierarce |
`commentTitle`, `postTitle`, `queryTitle`, `archiveTitle`. Also updates two small fallback instances where `postTitle` was used. Now uses `postContent` instead.
|
I removed the icons :) |
javierarce
approved these changes
May 3, 2022
This was referenced Oct 7, 2022
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.
We now have quite a few 'Title' blocks, the icons for which are all variations on a common theme; two horizontal lines accompanied by a letter. They look like this:
The letters already seem a little arbitrary, and I'm not sure how useful they are when we factor in i18n. This isn't a convention that will scale well if/when more 'Title' blocks are added.
It may be best to consolidate into a monolithic Title block, but that's a much bigger item. In this PR I just wanted to see if the usability is harmed if these blocks shared a single icon.
I used the existing
titleicon for convenience, but it may be better to refine the design. For now I'd like to get a sense of whether this is something worth pursuing.