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 keyboard outline in Kolibri - no longer showing unless keyboard navigating #227

Conversation

nucleogenesis
Copy link
Member

@nucleogenesis nucleogenesis commented May 5, 2021

Description

Issue addressed

Fixes learningequality/kolibri#7974

Steps to test

Kolibri

  • yarn link the design system (see KDS Readme for help there)
  • Run Kolibri and test that the outline comes up when you're keyboard navigating but does not come up when you're hovering with a mouse.

KDS

  • yarn dev the KDS documentation (or just use the Preview link below at the GIthub Actions)
  • Make sure that you see the keyboard navigation box when navigating by keyboard there.

(optional) Implementation notes

This commit is where the bug was introduced. @MisRob undoing this fixes the issue - was there some other effect you expected to see when making this change?

At a high level, how did you implement this?

Reverts this commit.

@nucleogenesis nucleogenesis added the product: Kolibri Relevant to a specific issue in Kolibri label May 5, 2021
@nucleogenesis nucleogenesis requested a review from MisRob May 5, 2021 19:20
@indirectlylit
Copy link
Contributor

was there some other effect you expected to see when making this change?

Looks like this was discussed and I OK'd the change here: #200 (comment)

Clearly I didn't (don't) fully understand how keyboard modality state is tracked and referenced. However it seems that we do in fact need both a JS and CSS-based state

@nucleogenesis nucleogenesis requested review from jonboiser and removed request for jonboiser May 11, 2021 13:04
@nucleogenesis
Copy link
Member Author

Thanks @indirectlylit

I'll merge this as there is low/no chance for regression

@nucleogenesis nucleogenesis merged commit 5b07ac2 into learningequality:v0.2.x May 11, 2021
@MisRob
Copy link
Member

MisRob commented May 12, 2021

Thank you for fixing this @nucleogenesis, I've had to misunderstand something. I still don't get how come that we rely on the attribute that I had removed but if the commit revert helps with the bug, there will be some dependency in Kolibri. Yes, I wouldn't expect any regressions.

@MisRob
Copy link
Member

MisRob commented May 12, 2021

Wondering if the problem might be rather some CSS specificity issue that has shown up after the selector update rather than the need to have the attribute there, because I think we actually don't update or use the attribute itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product: Kolibri Relevant to a specific issue in Kolibri
Projects
None yet
Development

Successfully merging this pull request may close these issues.

focus outline can appear around elements when hovering, even outside of keyboard modality
3 participants