-
Notifications
You must be signed in to change notification settings - Fork 16
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 state-dependent accordion background colors #1038
Implement state-dependent accordion background colors #1038
Conversation
@superstar54 @AndresOrtegaGuerrero please test this 🙏 I'm wondering how well it works. Also, please comment on the colors. Happy to change. Wasn't sure if |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1038 +/- ##
==========================================
+ Coverage 67.60% 67.65% +0.04%
==========================================
Files 117 117
Lines 6488 6498 +10
==========================================
+ Hits 4386 4396 +10
Misses 2102 2102
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
feb9d0c
to
e3fafad
Compare
src/aiidalab_qe/common/widgets.py
Outdated
@@ -1160,6 +1160,7 @@ def __init__(self, model: QWSM, **kwargs): | |||
super().__init__(children=[self.loading_message], **kwargs) | |||
self._model = model | |||
self.rendered = False | |||
self._bg_class = "" |
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.
what does "bg" stands for ?
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.
Standard short for background in CSS classes, but since it's not immediately clear, maybe I change it.
@edan-bainglass I tested , it looks nice , i dont have any preference regarding the colors you choose (i liked them) i just had some comments in the code. |
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.
LGTM! , thanks @edan-bainglass , lets see if @superstar54 is ok with the colors?
81800ed
to
def74fe
Compare
For the sake of time, I'll proceed. We can modify colors later if necessary. |
This PR uses the experimental
:has
CSS selector to apply background colors to the accordion item headers bases on the state of the accordion item's content (the wizard steps). If:has
is not supported by the browser (old version), the colors simply do not apply.Closes #989