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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dark tabs #4817

Merged
merged 20 commits into from
Jul 26, 2023
Merged

Dark tabs #4817

merged 20 commits into from
Jul 26, 2023

Conversation

lyubomir-popov
Copy link
Contributor

@lyubomir-popov lyubomir-popov commented Jun 30, 2023

Done

Add dark theme to tabs. Remove hover backgrounds, replace it by showing the tab highlight on hover. This simplfies theming and we should do it wherever possible.

Fixes WD-4680

QA

  • Open demo
  • [Add QA steps]
  • Review updated documentation:
    • /docs/patterns/tabs - review the dark tab part

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:

  • PR should have one of the following labels to automatically categorise it in release notes:
    • Feature 馃巵, Breaking Change 馃挘, Bug 馃悰, Documentation 馃摑, Maintenance 馃敤.
  • Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
    • if CSS class names are not changed it can be bugfix relesase (x.x.X)
    • if CSS class names are changed/added/removed it should be minor version (x.X.0)
    • see the wiki for more details
  • Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.

Screenshots

image

@bartaz bartaz changed the base branch from main to vanilla-4.0 June 30, 2023 13:22
scss/_patterns_tabs.scss Outdated Show resolved Hide resolved
scss/_patterns_tabs.scss Outdated Show resolved Hide resolved
scss/_patterns_tabs.scss Outdated Show resolved Hide resolved
@bartaz
Copy link
Contributor

bartaz commented Jun 30, 2023

I'm not sure if having active and hover state as the same turns out good. It looks a bit weird and confusing especially when you hover on tabs next to the one currently active making it "connected" with active bottom border:

tabs-hover

Also this makes tabs behave differently from top navigation, that still have hover/active states different:

tabs-hover-nav

@bartaz
Copy link
Contributor

bartaz commented Jun 30, 2023

Demo URL: https://vanilla-framework-4817.demos.haus/docs/patterns/tabs

(please note, for now demos need to be started and restarted manually every time we want them updated)

@lyubomir-popov
Copy link
Contributor Author

lyubomir-popov commented Jun 30, 2023

@bartaz that's a good point. I changed it to use the border default colour to make them different. I think this would be nice to also apply to side nav, main nav and in general everywhere we have a highlight bar as an indication of active state. Simpler and cleaner. But let's do one step at a time.

image
image

@bartaz
Copy link
Contributor

bartaz commented Jul 3, 2023

@lyubomir-popov Now, because of transparency of lines it shows "double" lines on hover (with thin line being visible as well as highlight):

image image

Is this the way we want to keep it?

@lyubomir-popov
Copy link
Contributor Author

Is this the way we want to keep it?

I'll fix it

@bartaz
Copy link
Contributor

bartaz commented Jul 6, 2023

@lyubomir-popov I had a quick look into moving the border by 1px. We can't move the bottom border down by 1px, because tabs have overflow hidden, so it would dissapear. We can try to move the thicker line 1px up - this would not affect baseline grid. But because active tab colour is different than bottom border colour the active tab then has a "double" border:

tabs-hover-1px

Although, I guess it's barely visible on standard zoom.

@lyubomir-popov
Copy link
Contributor Author

I think that's ok.

@ClementChaumel
Copy link
Contributor

Any update on this?

@lyubomir-popov
Copy link
Contributor Author

@ClementChaumel I agree with @bartaz's suggestion above, please implement that.

@ClementChaumel ClementChaumel added the Feature 馃巵 New feature or request label Jul 24, 2023
@ClementChaumel ClementChaumel changed the base branch from vanilla-4.0 to main July 24, 2023 13:17
scss/_patterns_tabs.scss Outdated Show resolved Hide resolved
scss/_patterns_tabs.scss Outdated Show resolved Hide resolved
ClementChaumel and others added 3 commits July 25, 2023 10:12
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
Copy link
Contributor

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@ClementChaumel ClementChaumel merged commit f939f4c into main Jul 26, 2023
6 checks passed
@ClementChaumel ClementChaumel deleted the dark-tabs branch July 26, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants