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

👌 IMPROVE: Styling for tabs #21

Merged
merged 4 commits into from
Oct 19, 2021

Conversation

pradyunsg
Copy link
Member

  • Introduce "hover" state styling for tabs. The lifecycle of a tab is
    now "inactive -> hover -> active".
  • Completely unmix styling for different states of a tab.
  • Use a higher contrast color for inactive tabs.
  • Increase width of overline, to match tab label underlines.

@welcome
Copy link

welcome bot commented Aug 15, 2021

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@codecov-commenter
Copy link

Codecov Report

Merging #21 (25d24bb) into main (6930a00) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #21   +/-   ##
=======================================
  Coverage   88.82%   88.82%           
=======================================
  Files          10       10           
  Lines         859      859           
=======================================
  Hits          763      763           
  Misses         96       96           
Flag Coverage Δ
pytests 88.82% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6930a00...25d24bb. Read the comment docs.

@pradyunsg pradyunsg marked this pull request as ready for review August 15, 2021 10:55
@chrisjsewell
Copy link
Member

chrisjsewell commented Aug 17, 2021

To compare:

Current (alabaster theme):

image

(on hover)

image

PR (alabaster theme):

image

(on hover)

image

https://squidfunk.github.io/mkdocs-material/reference/content-tabs (the original inspiration)

image

(on hover)

image

another point of reference is https://material-ui.com/components/tabs/#customized-tabs:

image

@chrisjsewell
Copy link
Member

chrisjsewell commented Aug 17, 2021

Thanks for the RP, given the above,

Use a higher contrast color for inactive tabs.

yep fair

Introduce "hover" state styling for tabs.

This is fine, but currently the change on hover is not really noticeable any more. I would still use (almost) the same active label color for hovering.

Increase width of overline, to match tab label underlines.

I disagree with this one; the selected tab underline is intended to be thicker than the rest of the line

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

see comments

@chrisjsewell
Copy link
Member

Hey @pradyunsg just giving a gentle nudge, to get your thoughts on my feedback 😬

- Introduce "hover" state styling for tabs. The lifecycle of a tab is
  now "inactive -> hover -> active".
- Completely unmix styling for different states of a tab.
- Use a higher contrast color for inactive tabs.
A more neutral colour to fit in better with the rest of the content.
As part of addressing review feedback.
@pradyunsg
Copy link
Member Author

(no hover)
Screenshot 2021-10-17 at 15 00 00
(hover)
Screenshot 2021-10-17 at 15 00 06

@chrisjsewell
Copy link
Member

Looks great cheers!

@chrisjsewell chrisjsewell merged commit fab8233 into executablebooks:main Oct 19, 2021
@pradyunsg pradyunsg deleted the tweak-tabs branch October 19, 2021 09:48
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.

3 participants