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

Fixes multiple-select in darkmode #2764

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

asizon
Copy link
Member

@asizon asizon commented Jan 19, 2022

Fixes muiltiple-select hover in darkmode. This plugin is used for new Presets feature.

Before:
nok

With this PR:
ok

@asizon asizon self-assigned this Jan 19, 2022
@asizon asizon added this to the 10.8.0 milestone Jan 19, 2022
haslinghuis
haslinghuis previously approved these changes Jan 19, 2022
@haslinghuis haslinghuis added this to For discussion in Finalizing Firmware 4.3 Release via automation Jan 19, 2022
@haslinghuis haslinghuis moved this from For discussion to Configurator in Finalizing Firmware 4.3 Release Jan 19, 2022
haslinghuis
haslinghuis previously approved these changes Jan 19, 2022
src/css/dark-theme.css Show resolved Hide resolved
@@ -73,6 +73,10 @@
display: inline-block;
}

.ms-drop ul>li.hide-radio:focus, .ms-drop ul>li.hide-radio:hover {
Copy link
Member

Choose a reason for hiding this comment

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

could you please add something like:

#presets_detailed_dialog
.ms-drop ul>li.hide-radio:focus, .ms-drop ul>li.hide-radio:hover {

so that this style has been applied only for this particular dialog.

Copy link
Member Author

Choose a reason for hiding this comment

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

Umm, really needed? Its already on presets_detailed_dialog css file so only works here.

Copy link
Member

@limonspb limonspb Jan 20, 2022

Choose a reason for hiding this comment

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

this css file is included from main html, so all the styles from this .css file will be applied to the whole Configurator. Unless you narrow them with more specific selectors.

Copy link
Member Author

@asizon asizon Jan 20, 2022

Choose a reason for hiding this comment

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

Ouch! Understood, I thought that it is included amd comes only in presets dialog html and js files.

remove hover background from presets dialog

add specific selector
@sonarcloud
Copy link

sonarcloud bot commented Jan 20, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@asizon
Copy link
Member Author

asizon commented Jan 20, 2022

Done!Ready for merge

@asizon asizon added the Tested label Jan 20, 2022
@blckmn
Copy link
Member

blckmn commented Jan 20, 2022

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

Finalizing Firmware 4.3 Release automation moved this from Configurator to Firmware Jan 21, 2022
@asizon asizon merged commit 8aa3a74 into betaflight:master Jan 21, 2022
Finalizing Firmware 4.3 Release automation moved this from Firmware to Finished (Merged) Jan 21, 2022
@haslinghuis haslinghuis moved this from Finished (Merged) to Configurator in Finalizing Firmware 4.3 Release Jan 22, 2022
@haslinghuis haslinghuis moved this from Configurator to Finished (Merged) in Finalizing Firmware 4.3 Release Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants