canonical / vanilla-framework

From community websites to web applications, this CSS framework will help you achieve a consistent look and feel.
https://vanillaframework.io
GNU Lesser General Public License v3.0
850 stars 169 forks source link

Add theme support to tooltip component #5326

Closed jmuzina closed 2 months ago

jmuzina commented 3 months ago

Done

Fixes WD-11871

QA

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

Screenshots

image

Screenshot 2024-08-30 at 10 19 36 AM
webteam-app commented 3 months ago

Demo

Jenkins

demos.haus

bartaz commented 3 months ago

The contrast between dark tooltip and dark background doesn't seem sufficient. Tooltip is barely visible.

image

@lyubomir-popov Any thoughts? Not sure how often tooltips are used on top of dark backgrounds yet (so we may not need to address this right away), but this doesn't seem to be a good long term solution.

Or maybe the issue is the contrast ratio between dark and alternative dark backgrounds? But obviously changing those will affect many other places.

lyubomir-popov commented 3 months ago

Or maybe the issue is the contrast ratio between dark and alternative dark backgrounds? But obviously changing those will affect many other places.

let's just flip to the opposite theme in tooltips, iirc lots of people felt that to be the right choice last time we looked at these.

jmuzina commented 3 months ago

Or maybe the issue is the contrast ratio between dark and alternative dark backgrounds? But obviously changing those will affect many other places.

let's just flip to the opposite theme in tooltips, iirc lots of people felt that to be the right choice last time we looked at these.

@lyubomir-popov This has been updated to invert the tooltip color themes.

jmuzina commented 2 months ago

Overall, the theming functionality looks good now, but that disconnected tail seems like a thing we should fix here, if possible.

@pastelcyborg @bartaz and I discussed this briefly last week and I forgot to note it here: this only seems to happen when you zoom the page beyond 100%. Does it occur on other magnifications for you? If so, on what browser(s)?

bartaz commented 2 months ago

Detached tooltip is not white?

image
jmuzina commented 2 months ago

Detached tooltip is not white?

@bartaz

I think they are the same #f7f7f7 - maybe the different background contrast makes them look different?

Screenshot 2024-09-04 at 9 25 20 AM Screenshot 2024-09-04 at 9 25 30 AM

@lyubomir-popov can you verify whether these tooltips look correct?

jmuzina commented 2 months ago

@bartaz @pastelcyborg Updated this to support & document theme inversion override as discussed today:

Screenshot 2024-09-04 at 2 41 04 PM
jmuzina commented 2 months ago

@pastelcyborg @bartaz Do you think this is more clear semantically if the .p-tooltip with overridden background uses the .is-<theme> of the background that is actually displayed for the tooltip? I wonder if it's a bit confusing that .p-tooltip.is-light results in a tooltip with a dark background

pastelcyborg commented 2 months ago

@pastelcyborg @bartaz Do you think this is more clear semantically if the .p-tooltip with overridden background uses the .is-<theme> of the background that is actually displayed for the tooltip? I wonder if it's a bit confusing that .p-tooltip.is-light results in a tooltip with a dark background

I would stick with the solution we've come up with. The theme values represent the component when used with a particular theme, not the current color scheme most visually prevalent within the component.

lyubomir-popov commented 2 months ago

@jmuzina looks good, only spotted one thing - the incorrect example at the bottom works the same way as the correct one:

image

https://vanilla-framework-5326.demos.haus/docs/examples/patterns/tooltips/combined?theme=light

jmuzina commented 2 months ago

@jmuzina looks good, only spotted one thing - the incorrect example at the bottom works the same way as the correct one: image

https://vanilla-framework-5326.demos.haus/docs/examples/patterns/tooltips/combined?theme=light

@lyubomir-popov It depends on the theme of the document body, try switching the page theme and you'll see it changes.

The one with the dark strip only changes between the two tooltips when the page itself is light-themed