-
-
Notifications
You must be signed in to change notification settings - Fork 531
Resource Tree Icons Adjustment and Contrast Increase #16761
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
base: 3.x
Are you sure you want to change the base?
Resource Tree Icons Adjustment and Contrast Increase #16761
Conversation
Switched Unpublished and Deleted Resource Icons in the Tree to use the outline version of the Font Awesome icon set for the Resource/Template if one exits.
Made the icons for Resources that were published the same for both hidden and visible.
Improved contrast for tree to enhance readability for the tree text.
|
I love using the outlined icons for unpublished resources, by why not keep the grayed-out versions for published & hidden ones? That gives an extra visual cue beyond just the italic text. |
|
@SnowCreative - I think the idea is to avoid any crossover of meaning, which would require the gray be used to either indicate unpublished or hidden. In this case the choice was for gray to be the visual cue for unpublished in addition to the new outline style, and that makes sense to me. |
|
@jaygilmore Thanks for the implementation, I like this option, it seems the logical and does not require special code edits (although changing icons adds unnecessary visual states). |
@smg6511 Wow, that would be great. I'd love to be able to change visible status just by clicking a little icon in the tree, in addition to that icon showing the visibility status so we don't need italics for anything. I have a whole set of sites where admins are constantly updating the visibility status of pages, and this would make it so they don't have to load the whole page for editing just to do that. You've also introduced another visual element — color. I've thought of using that in the past, and actually added some CSS to one site to color the titles and icons depending on status. That makes it super-easy to to see status without needing italics, although multi-colored titles might not be to everyone's liking. |
This thought crossed my mind too; the free FA5 version only gives you 89 of the 672 icons in outline form. The basic ones we need are there though. We may want to consider a future switch to Google's Material Symbols font, as it offers a lot more flexibility at no cost. |
This is a fair point. @rthrash felt unpublished needed a stronger visual cue. @opengeek and @rthrash and I discussed that hidden is a meta value and only matters in the cases where and extra actually chooses to. Also, if you want a plugin that preserves/returns the exact 2.x standard, I have a POC for that and could update it to do just that. I can have it in a day or so. Just say the word, @SnowCreative. @smg6511 I like the problem you're aiming to solve. I do think adding more icons has some potential downsides in terms of cognitive load and usability around a lot of little click targets in a small space but it does provide more feedback at a glance also. I'm not sure if toggling menu visibility in the tree makes a tonne of sense where pub/unpub would always make sense to me. When I'm building a site I set menu visibility to off if it will be omitted but rarely after that point. Publishing is something I do more often—especially from a published to unpublished state. In any event, it's probably worth a standalone proposal. |
|
@jaygilmore - Re your feedback to my proposed idea: Yeah, I definitely factored in the question of "is this going to be too much visual input" as I was thinking through this. It'd have to be actually prototyped in a proposal PR to really work it through. On the menu show/menu hide toggling, it'd simply be a nice feature for those who might do that a lot; you don't, Michael does ... just a matter of one's work style and nature of the sites they handle. We already have the contextual menu for pub/unpub; not as quick as a toggle button, but still a convenience that's already there. Re your PR, I'm good with what it's doing, but you'll need to go about it differently to cover the issue that @Ruslan-Aleev raised (that only a small subset of solid icons are available in outline form). Take the file I've attached for a spin and see what you think. (I couldn't make suggested changes in the PR itself, as some of my changes are too far outside of the scope of yours.) |
|
@jaygilmore Jay - giving you a little bump here. Did you get a chance to take a look at the solution in the file I attached? |
I'm going to look at this tomorrow-ish. I don't get these notifications as I should. |
|
@jaygilmore - I see you had eyes on this PR yesterday - I wanted to remind you to check out the file I attached with suggested changes ;-) |
|
@smg6511 I started looking at your file and comparing mine. I can't see any difference in the results. Font weights are ignored if unsupported by the font and the next state is shown. So, if I switch to an icon that is only solid, the icons are still shown, when I use my file vs yours. Let me know if I'm missing something or if you think there's anything I should test that I didn't?
|
|
@jaygilmore - I get the feeling you may have run rebuild to inspect my updates but then did not rebuild again when you went back to compare them to yours (??). Or maybe you have something special in your environment that is handling missing icons? Whatever the case, there's really no falling back in terms of the FA icons (at least not out of the box); if a "weight" of a specific icon isn't in the free set the icon will not show. Here are a couple screenshots from my end to illustrate: |
|
@smg6511 I'll take another look. This is really strange that the icons are not showing at the lower weights—unless the font supports the lower weight and we just didn't include it in the set we have with Revo. Out of curiosity, what browser were you using for testing? |
|
@jaygilmore - In my experience here and elsewhere there's no such thing as FA fonts falling back to an available version; if you request a pro-level icon and only have free-level access you'll get no icon. But maybe you've seen different behavior. FWIW I don't think you've forgotten to include the available outline fonts; you can always verify on the FA site, which is what I did when taking a look at this. In any case, I do most dev on Chrome/Mac (latest, or near latest version). |
|
@smg6511 I figured it out, indeed, it's Chrome not behaving as Safari. The spec for fonts (including any glyph) is to match the weight with the next available weight and if not it should synthesize it. This is what Safari is doing. This is not what Chrome is doing. Anyhow, your fixes make sense. I'm not quite sure what the process might be to handle this for each font class. For example, (as shown in the image below) the deleted item icon vanishes as well.
I'd be happy to figure out how to incorporate these but I could use your guidance, Jim. :) |
|
@jaygilmore - Oh, interesting (re Safari doing swaps). I can take a closer look at dealing with other missing glyphs in the next couple days. I think we will want to stick to whitelisting the supported outline classes that are spec'd/needed by the core, instead of doing it for the entire supported subset (roughly 89 glyphs). Cheers ;-) |












What does it do?
Differentiates Unpublished/Deleted Content from Published via the icons. Publish uses solid, unpublished/deleted uses outline versions. This helps folks quickly identify resources that are published to work on since unpublished and deleted are not generally active.
Darkens the colors of the text for both the normal, unpublished and deleted item Resource Tree text to improve readibility and accessibility.
Why is it needed?
The 3.x implementation of the tree adjustment was generally an improvement that brought a logical change to the text colors, but it made it slightly harder to identify published resorurces due to the Hidden from menu items being greyed out—even if published.
Screenshots
How to test
Check out this PR and run the build.
Related issue(s)/PR(s)
This is an alternative to #16760 and replaces #16759 entirely by retaining the changes done originally by @Ruslan-Aleev in #14832. This is an alternative implementation that should also adequately address the issue in #16465.