-
Notifications
You must be signed in to change notification settings - Fork 81
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 outlines for KTextbox and KSwitch #191
Fix outlines for KTextbox and KSwitch #191
Conversation
@indirectlylit From here:
I double-checked with both clicking by mouse or tabbing with the keyboard and it seems to shift with focus, which I think means that it is validation-related. I will open a separate issue for this problem. |
That sounds right, thanks! One question though – I thought we'd switched textboxes to the new style in #167 These look like it's still using the old styling? (cc @nucleogenesis who worked on the other work) |
Now it will be fixed here: #240 @indirectlylit |
@indirectlylit With the new style for |
hooray! thanks guys |
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.
Thank you @sairina, code looks good to me, and I haven't noticed any problems when testing locally.
Description
Issue addressed
Addresses #131
Before/after screenshots
KTextbox
BeforeKTextbox
AfterKTextbox
(after background fixed)KSwitch
BeforeKSwitch
AfterSteps to test
KTextbox
KSwitch
/en/
to/ar/
for example.(optional) Implementation notes
At a high level, how did you implement this?
For both of these components, I added
:style
and a ternary for whether or not there was a focus on the element. I also made updates totrackInputModality.js
in order to differentiate between keyboard tab and mouse click.Does this introduce any tech-debt items?
This in itself does not introduce any tech-debt items. However, it should be noted that there is a new CSS pseudo-class called
:focus-visible
that simplifies a lot of the logic that was introduced in order to track the use of keyboard vs. mouse, and it will likely be a refactor that we will want to look into in the design system. I will be opening an issue and linking this issue.Reviewer guidance
Comments
I accidentally forgot and added both
KTextbox
andKSwitch
together in this PR 😩