Skip to content
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

Remove the color:inherit behavior from sidebar folder collapsed state. #8538

Closed
3 of 6 tasks
Truncated opened this issue Nov 18, 2022 · 4 comments
Closed
3 of 6 tasks
Assignees
Labels
bug Functionality which is not working as intended ui Issues focused on user interface improvements

Comments

@Truncated
Copy link

What happened?

In the journal folder list, the backgrounds are selectable. The font color is not, and should automatically adjust to a high-contrast option based on the background color for accessibility purposes.

While this does function on the main journal folder, when using Monk's Enhanced Journal, I found that the collapsed folders did not adjust their font color appropriately - the variable style was being usurped in the evaluation.

What ways of accessing Foundry can you encounter this issue in?

  • Native App (Electron)
  • Chrome
  • Firefox
  • Safari
  • Other

Reproduction Steps

  1. Make a couple folders with different color (one dark and one light) backgrounds at least two levels deep.
  2. Install Monk's Enhanced Journal
  3. View folders in journal
  4. Squint at it
  5. Open Dev Tools and highlight a collapsed folder header
  6. for li.folder.collapsed > .folder-header h3, uncheck "color: inherit;" (in style.css:3119)

This allows li.folder > .folder-header h3 to take the color value, which restores the dynamic setting for contrast option.

What core version are you reporting this for?

10.290

Relevant log output

No response

Bug Checklist

  • The issue occurs while all Modules are disabled
@Truncated Truncated added the bug Functionality which is not working as intended label Nov 18, 2022
@Truncated
Copy link
Author

Truncated commented Nov 18, 2022

Demonstration of visual issues
(With inherit in place - current experience)
image
(removed color from class li.folder.collapsed > .folder-header h3)
image

@Fyorl
Copy link
Contributor

Fyorl commented Nov 18, 2022

Auto-contrasting the folder names with the background colour is a good idea, but as far as I can tell, we don't have any such existing styling. For example, if I create a white folder in the sidebar, it still has white text:

image

It seems in the case of this module-provided sidebar, it's simply inheriting the wrong colour, perhaps? The icons are also black rather than white, for example. I'd have to install the module myself to be certain.

@Truncated
Copy link
Author

Auto-contrasting the folder names with the background colour is a good idea, but as far as I can tell, we don't have any such existing styling. For example, if I create a white folder in the sidebar, it still has white text:

This is a good point. I went back and did some more troubleshooting with the base Foundry and then with Enhanced Journal, to isolate variables. I learned three things:

  1. I found that what I was interpreting as variable generation (due to assuming from the var statement with no enough time to dig) was actually a variable call for a single hard-coded color; a flavor of off-white: --color-text-light-highlight: #f0f0e0; from line 5 of style.css. This means that all folder headers are hard-coded as this off-white - full stop.
  2. The collapsed value of color: inherit on line 3121 of style.css in Foundry has no impact on the core Folder display of the side bar. I didn't do full impact testing, but I'm fairly certain it has no adverse impact without some distinct misuse of the class.
  3. When using Monk's Enhanced Journal, however, it does have an impact as it blocks the primary color from li.folder > .folder-header h3 from going into effect properly.

So I think the ask is to remove color:inherit from line 3121 of style.css in the next patch, which will at least let all the folder text to maintain the same light color in core AND Monk's (and other folder-related things). Since the issue is the font is hard-coded, choosing background colors allows the user to mitigate the contrast issue for now.

Bigger picture, the structure of the current CSS is very brittle. Shifting this to a light/dark series of color options for text (added as a customizable setting in the main config menu), which then is employed if the background for any element based on the 50% mark of lightness calculation throughout. This could be done through CSS or JS to pass / detect the background colors, but that would allow it to be surgically fixed throughout without having to replace so much of the existing code.

Better would replace a lot of the first CSS lines in style.css with variables of HSL values depending on the purpose, rather than directly hard-coding colors. This would allow easier changes of colors in themes to flow better in relation to different parts, but I wouldn't want to wait on that to get at least consistant folder text color mentioned above

@Fyorl
Copy link
Contributor

Fyorl commented Nov 21, 2022

We can remove the color: inherit if it's indeed not serving any purpose here. I will shift it to v11 in case there are modules or systems relying on this behaviour.

@Fyorl Fyorl added the ui Issues focused on user interface improvements label Nov 21, 2022
@aaclayton aaclayton changed the title Collapsed h3 css includes color:inherit Remove the color:inherit behavior from sidebar folder collapsed state. May 1, 2023
@aaclayton aaclayton added this to the Version 11 - Testing 2 milestone May 1, 2023
@Fyorl Fyorl self-assigned this May 2, 2023
@Fyorl Fyorl closed this as completed May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Functionality which is not working as intended ui Issues focused on user interface improvements
Projects
Status: Done
Development

No branches or pull requests

3 participants