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

Fix highlighting on history panel #1698

Merged
merged 4 commits into from Nov 3, 2022

Conversation

scuml
Copy link
Contributor

@scuml scuml commented Nov 2, 2022

#djDebug .djdt-panelContent tbody > tr:nth-child(odd) is overriding #djDebug .djdt-highlighted on the history panel so every odd row will not take on a highlighted appearance.

@tim-schilling
Copy link
Contributor

Thanks for the PR!

I'm not a fan of using !important we should be able to design our css in a way that avoids that since it's pretty small. Can you give us the scenario that produces the effect this fixes?

And as a side note, the commit message here should contain the description you used in the PR. That description provides a fair amount of context of why the change was made and would be beneficial in our git history.

@scuml
Copy link
Contributor Author

scuml commented Nov 2, 2022

Yeah - not a fan of !important myself, but thought this might be a good use of it. Check out the history panel - every odd row for me does not highlight when switch is pressed.

Screen Shot 2022-11-02 at 3 12 40 PM

@scuml
Copy link
Contributor Author

scuml commented Nov 2, 2022

It looks like our options are !important or the awkward...

#djDebug .djdt-highlighted, #djDebug .djdt-panelContent .djdt-highlighted:nth-child(odd){
    background-color: lightgrey;
}

@tim-schilling
Copy link
Contributor

Hmm. Yeah, I'm not sure. It's not terrible with a linebreak:

#djDebug .djdt-highlighted,
#djDebug .djdt-panelContent .djdt-highlighted:nth-child(odd){
    background-color: lightgrey;
}

Another option is to use the :not() operator, #djDebug .djdt-panelContent tbody > tr:nth-child(odd):not(.djdt-highlighted)

@scuml
Copy link
Contributor Author

scuml commented Nov 2, 2022

Good thought on :not() - that seems more sensible.

@tim-schilling tim-schilling merged commit a481849 into jazzband:main Nov 3, 2022
@tim-schilling
Copy link
Contributor

Thank you for the PR! This is a nice fix to have.

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.

None yet

2 participants