-
Notifications
You must be signed in to change notification settings - Fork 273
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
feat(ui5-button): implement sap_horizon theme #4126
Conversation
@@ -9,4 +9,5 @@ | |||
--_ui5_button_icon_font_size: 1rem; | |||
--_ui5_button_emphasized_font_weight: bold; | |||
--_ui5_button_text_shadow: none; | |||
--sap_button_emphasized_focused_border: 0.0625rem dotted var(--sapContent_ContrastFocusColor); |
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.
all params start with --_ui5_button, not --sap_button
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.
The focus on fiori_3 regular toggle button, when in active state is invisible. |
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.
There is a misalignment between two focuses of the emphasized button
Can you check if the focus color when button is pressed down is correct in Fiori_3 (and FIori_3 Dark as well), because it changed with the current PR, but I guess it should not. Which of the two hover states over a default type button is correct |
@ilhan007 The focus is indeed broken in the PR, the colors on press should be inverted (black to white). |
The hover should only apply shadow. the blue text color is applied when the button becomes active. |
After the last changes, it looks good. Checked the visual and with ui5. I think the button is in good state at the moment, and the comments made, were fixed. @ilhan007 |
All right, I won't test further, you can merge the PR whenever you decide |
feat(ui5-button): implement sap_horizon theme feat(ui5-toggle-button): implement sap_horizon theme
This reverts commit 327504e.
feat(ui5-button): implement sap_horizon theme
feat(ui5-toggle button): implement sap_horizon theme