-
Notifications
You must be signed in to change notification settings - Fork 3
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
Implement accordion preservation action task lists #1103
Conversation
e61d9bd
to
f9ef799
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1103 +/- ##
==========================================
+ Coverage 54.66% 54.69% +0.02%
==========================================
Files 105 105
Lines 7696 7696
==========================================
+ Hits 4207 4209 +2
+ Misses 3229 3228 -1
+ Partials 260 259 -1 ☔ View full report in Codecov by Sentry. |
f9ef799
to
3c6896a
Compare
Here's how it looks in action. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice, great work @djjuhasz!
dashboard/src/styles/main.scss
Outdated
.accordion-button:hover { | ||
background-color: $primary-bg-subtle; | ||
border-color: $primary; | ||
border-width: 2px; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also target the focus state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the focus state actually changes the way the highlighting works. The current behaviour is that the "
active" (i.e. open) section has a different look (purple header text, purple arrow icon, white background) then the closed sections (black text, black arrow icon, light gray background), which is different from the hover style above. Because the accordion button gets focus when it's clicked, and it doesn't lose focus until you click somewhere else, it effectively becomes the "active" style for the section unless you tab or change the focused element some other way. This change isn't bad, but I think it makes it a bit less obvious that preservation action header is an interactive element when another section is clicked.
Here's an screencast of how it the accordion works with the focus state added:
Screencast from 2025-01-15 10-19-54.webm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could set in the selector focus + not active. Not a big deal in any case, I just think it's nice to also have a visual hint when doing keyboard navigation.
Refs issue #986 Use the Bootstrap 5 accordion component to implement collapsable preservation task lists. The accordion component supports using a large button as a section header and closing open sections when a new section is opened. The specific #986 features implemented are: - Add a border around the preservation action header to hint that it is a control widget - Make the entire preservation action header clickable to collapse or expand the related task list - Change the preservation action header border and background colors on hover
3c6896a
to
00a91e9
Compare
Refs issue #986
Use the Bootstrap 5 accordion component to implement collapsable preservation task lists. The accordion component supports using a large button as a section header and closing open sections when a new section is opened.
The specific #986 features implemented are: