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

Adding text-shadow to time/rez classes #647

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheTacoScott
Copy link

Makes the time and rez metadata display exceedingly more readable.

Copy link
Owner

@whyboris whyboris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR 😁

I do dislike that there are times when the time & resolution are hard to see (when the background of the screenshot matches too much with the text). But I'm unsure how to best solve this problem.

I am rather against text-shadow as it is "expensive" performance-wise. Especially if it's such a complicated text shadow.

A semi-transparent background on the text element would be much less performance-heavy - but it doesn't look very good. Adding a border-radius is also "expensive" but would likely be needed to make it look better.

I'm unsure about how to visually address the problem I've had in the app since version 1 😅

I'm open to a discussion. Feel free to share screenshots 🤝

@TheTacoScott
Copy link
Author

TheTacoScott commented Mar 10, 2021

Background where it's addition doesn't matter much, but also is no worse off.
image

@TheTacoScott
Copy link
Author

Good example of when it is basically unreadable without shadow
image

@TheTacoScott
Copy link
Author

Much easier with shadow:
image

@TheTacoScott
Copy link
Author

Unreadable:
image

Readable:
image

@TheTacoScott
Copy link
Author

Those are the readability improvement screenshots. I think there is a case to improve the readability. But you bring up a very good and valid point: what is the right way to do it?

Outside stroke (aka: text shadow 8 axis) makes it pop a bit, but it's not a free operation in css.
Span/Div Opacity changes make it all look a bit too boxy for my taste.

here is an example of adding some padding/and setting a background color and some opacity:
image

@TheTacoScott
Copy link
Author

With a border radius:
image

@TheTacoScott
Copy link
Author

Fairly easy to get a computationally cheap CSS stroke effect with text-stroke (interior stroke) but then the font size has to be bumped up a lot;
image

@TheTacoScott
Copy link
Author

mix-blend-mode might have some options:
difference:
image
image

@whyboris
Copy link
Owner

Thank you for the screenshots and ways to address the current problem. I'm a bit busy this coming month - but I'll return to this topic when I find the time again 🤝

@TheTacoScott
Copy link
Author

No rush. In fact I think the most proper way to solve this might be the same way you display the "total time of videos in folder" in folder view. It's a simple border radius box. While that is less 'cool' than fancy opacity/text shadows/etc it would be more in line with the existing visual language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants