Skip to content

Remove important keyword in focus opacity style#9700

Merged
vince-fugnitto merged 1 commit intoeclipse-theia:masterfrom
xcariba:remove-focus-opacity-important-keyword
Jul 19, 2021
Merged

Remove important keyword in focus opacity style#9700
vince-fugnitto merged 1 commit intoeclipse-theia:masterfrom
xcariba:remove-focus-opacity-important-keyword

Conversation

@xcariba
Copy link
Contributor

@xcariba xcariba commented Jul 5, 2021

Signed-off-by: Alexander Kozinko xcariba@gmail.com

What it does

Allows client to override opacity in focused elements without !important keyword.
I've tried to set existing style to my extension but there is a problem with opacity. There is no way to override it anywhere without marking every opacity change in css. I found this conversation and fixed it. I've checked existing styling and nothing changed. I also looked in all :focus overrides in this repository and found that it has a few overrides with this keyword. I think that it will be better if this base property can be changed without additional marks.

How to test

  • Open Preferences, set focus in Search Settings, Editor: Font Size, Editor: Render Whitespace and check that focus is visible. Set focus on Editor: Insert Spaces Checkbox using Tab key and check that focus border is visible.
  • Open debug view and use Tab to move focus on buttons, tree view headers and configuration selector, check that focus border is visible.
  • Open Find Command view (Ctrl+Shift+P) and check that focus border is visible.

Review checklist

Reminder for reviewers

@vince-fugnitto
Copy link
Member

@xcariba please fill out the how to test section for reviewers.

@vince-fugnitto vince-fugnitto added css Issues related to CSS functionality extensibility issues to simplify ability to extend Theia labels Jul 5, 2021
@xcariba
Copy link
Contributor Author

xcariba commented Jul 5, 2021

@xcariba please fill out the how to test section for reviewers.

Added steps to check that existing focus selector is visible. Please, let me know if I need to add anything or check to ensure that nothing is broken.

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

I couldn't find any regressions - i.e. no focused element which was missing its borders - due to the removed !important statement.

So LGTM then.

Signed-off-by: Alexander Kozinko <xcariba@gmail.com>
@xcariba xcariba force-pushed the remove-focus-opacity-important-keyword branch from cb917c0 to 35a21b9 Compare July 16, 2021 17:23
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I'm fine with the changes, I did not notice any regression during testing 👍

@vince-fugnitto vince-fugnitto merged commit 63c6f02 into eclipse-theia:master Jul 19, 2021
@vince-fugnitto vince-fugnitto added this to the 1.16.0 milestone Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

css Issues related to CSS functionality extensibility issues to simplify ability to extend Theia

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants