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

Add default focus style to styled Checkbox #2099

Merged

Conversation

RoyEJohnson
Copy link
Contributor

@RoyEJohnson RoyEJohnson requested a review from a team as a code owner January 3, 2024 20:04
@RoyEJohnson RoyEJohnson requested a review from jivey January 3, 2024 20:04
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 3, 2024 20:05 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 5, 2024 16:59 Inactive
@@ -48,6 +48,11 @@ const Checkbox = ({children, className, ...props}: React.PropsWithChildren<Props
{children}
</label>;

const defaultFocusOutline = `
outline: 0.1rem dotted #212121;
outline: 0.5rem auto -webkit-focus-ring-color;
Copy link
Member

Choose a reason for hiding this comment

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

indent the styles by one please

@@ -48,6 +48,11 @@ const Checkbox = ({children, className, ...props}: React.PropsWithChildren<Props
{children}
</label>;

const defaultFocusOutline = `
outline: 0.1rem dotted #212121;
Copy link
Member

Choose a reason for hiding this comment

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

the dotted style is the firefox style, chrome has a blue gradient thing, is it possible to use the default focus styles here implicitly or do we have to use a custom style? or is that what you're doing by defining both the dotted and the webkit at the same time like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I depended on this StackOverflow post, which suggests that you have to spec it. initial does not work.

Copy link
Member

Choose a reason for hiding this comment

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

ah ok... yea i don't see a simpler solution here since we're doing a hidden-checkbox situation. you may want to just throw that stack overflow link in a comment above the defaultFocusOutline style

@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 5, 2024 22:07 Inactive
@RoyEJohnson RoyEJohnson force-pushed the add-highlight-color-to-search-highlight-aria-labels branch from 5b86f13 to c60b61e Compare January 5, 2024 23:50
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 5, 2024 23:50 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 8, 2024 14:33 Inactive
@RoyEJohnson RoyEJohnson force-pushed the add-highlight-color-to-search-highlight-aria-labels branch from c657567 to d9c839f Compare January 8, 2024 19:35
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 8, 2024 19:36 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 8, 2024 22:06 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 9, 2024 19:50 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 9, 2024 20:21 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 9, 2024 21:25 Inactive
@Malar-Natarajan
Copy link
Contributor

@RoyEJohnson The CI check Lint is failing in this PR. Could you please resolve. Thanks!

@RoyEJohnson RoyEJohnson force-pushed the add-highlight-color-to-search-highlight-aria-labels branch from 9083e69 to ccc4049 Compare January 10, 2024 14:01
@TomWoodward TomWoodward temporarily deployed to rex-web-add-highlight-c-k1boav January 10, 2024 14:02 Inactive
@Malar-Natarajan Malar-Natarajan merged commit 9a33b54 into main Jan 10, 2024
9 of 11 checks passed
@Malar-Natarajan Malar-Natarajan deleted the add-highlight-color-to-search-highlight-aria-labels branch January 10, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants