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: no button outline #3240

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

fix: no button outline #3240

wants to merge 1 commit into from

Conversation

wenfangdu
Copy link

@wenfangdu wenfangdu commented Jul 29, 2024

Fix

Button outline looks bad, not sure why it's added:

iShot_2024-07-29_19.47.21.mp4

Test

  1. Click the note entry
  2. No button outline
iShot_2024-07-29_19.54.45.mp4

Release

Removed button outline.

@roundhill
Copy link
Contributor

Thanks for the PR @wenfangdu! Agreed, that looks weird when you click on the rows. Unfortunately this change will remove the outline for keyboard navigation. I think we can use the modern focus-visible CSS selector though to fix this so it doesn't show the outline for mouse clicks. Something like:

button {
  background: transparent;
  border: none;
  padding: 0;
  cursor: pointer;
  font-size: 14px;
  appearance: none;
  vertical-align: baseline;
  outline: none;

  svg {
    cursor: pointer;
  }

  &:focus-visible {
    outline: $focus-outline;
  }
}

And then we'd want to remove the focus-visible package from package.json and remove all of the @extend %smart-focus-outline; lines along with the smart-focus-outline styles themselves. Are you up for making this change?

@wenfangdu
Copy link
Author

@roundhill Sorry, haven't got time recently, I'm currently using custom css to hide the outlines, it's good for now, if anyone steps into this PR, you can refer to @roundhill's comment for guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants