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

refactor: replace slow performing CSS selectors #2968

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

mfranzke
Copy link
Member

@mfranzke mfranzke commented Aug 3, 2024

Proposed changes

Resolves #2967

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (fix on existing components or architectural decisions)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Further comments

@mfranzke mfranzke added the 🍄🆙improvement New feature or request label Aug 3, 2024
@mfranzke mfranzke added this to the Guidelines 3.0 RC milestone Aug 3, 2024
@mfranzke mfranzke self-assigned this Aug 3, 2024
@mfranzke mfranzke linked an issue Aug 3, 2024 that may be closed by this pull request
@mfranzke mfranzke changed the title refactor: replaced by simpler solution refactor: replace slow performance CSS selectors Aug 3, 2024
Copy link
Contributor

github-actions bot commented Aug 3, 2024

🔭🐙🐈 Test this branch here: https://db-ui.github.io/mono/review/2967-slow-performance-css-selectors

@github-actions github-actions bot added the 🏘components Changes inside components folder label Aug 3, 2024
Comment on lines 85 to 87
> .db-navigation-item {
&[aria-current="page"] {
@extend %show-db-puls-auto;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would certainly improve performance. Unfortunately, through this change NavigationItems are no longer marked as active if a nested NavigationItem has aria-current=“page”. From a11y point of view, we recommend that you only set aria-current=“page” to the anchor that points to the current page.

An alternative would be to use JS to set an “active” class from the innermost active NavigationItem to the outermost NavigationItem. But is that better than using the has-selector...?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think to remember that this wasn't even only about line 84 within the original file, but especially about the upfollowing 88 till 96, that even also provided a concatenated :has selector. I'll further investigate on this one.

bruno-sch
bruno-sch previously approved these changes Aug 26, 2024
@mfranzke mfranzke changed the title refactor: replace slow performance CSS selectors refactor: replace slow performing CSS selectors Sep 22, 2024
@mfranzke mfranzke removed this from the Guidelines 3.0 RC milestone Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏘components Changes inside components folder 🍄🆙improvement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

slow performance CSS selectors
2 participants