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] Focus style align with style guide #324

Merged
merged 2 commits into from
May 7, 2020

Conversation

aappoalander
Copy link
Contributor

Description

Update for current focus styles so that they match the new style guide focus styles.

Motivation and Context

Current focus styles were following the design system specification which was never taken into use outside design system implementation. This was blocking ui-components usage in projects. Focus styles were redesigned to better match default styles that are in use in may services and applications.

How Has This Been Tested?

Tested using styleguidist and Create React App TypeScript version with long contents for components.

@aappoalander aappoalander added the enhancement New feature or request label May 7, 2020
@aappoalander aappoalander added this to the 1.0.0 milestone May 7, 2020
@aappoalander aappoalander requested review from ketsappi and LJKaski May 7, 2020 10:36
@aappoalander aappoalander self-assigned this May 7, 2020
@aappoalander aappoalander force-pushed the fix/focus-style-align-with-styleguide branch from d19b615 to 086b7bf Compare May 7, 2020 10:53
Copy link
Collaborator

@LJKaski LJKaski left a comment

Choose a reason for hiding this comment

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

Try it with the 4px radius. If it works, I'd say go with that value. If not, the current works just fine :)

@@ -1,4 +1,4 @@
export const radius = {
basic: '2px',
focus: '4px',
focus: '2px',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the 4px radius is closer to the current design if that's a viable value otherwise.

@aappoalander
Copy link
Contributor Author

Try it with the 4px radius. If it works, I'd say go with that value. If not, the current works just fine :)

The 4px radius does not work as well as in the designs. The distance from the component border does not remain constant so I'd stick with the current value.

@aappoalander aappoalander merged commit eb6c5ae into develop May 7, 2020
@aappoalander aappoalander deleted the fix/focus-style-align-with-styleguide branch December 4, 2020 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants