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

[DO NOT MERGE] Fix textbox being unexpectedly focused after the first page load #11290

Closed
wants to merge 1 commit into from

Conversation

MisRob
Copy link
Member

@MisRob MisRob commented Sep 22, 2023

@radinamatic @pcenov

This links Kolibri to this KDS PR learningequality/kolibri-design-system#449 which fixes #9077 and possibly other pages where focus ring appears around textbox on its own when a page loads.

This was caused by

So when ProfileEditPage is first loaded there is api call for user data, which then updates the textbox value which causes it to have coreOutline. It can confirmed this by stopping this api call, then the page will not have it when loaded. ~ @thesujai (#9077 (comment))

so it's particularly relevant to pages where we load some data for a text box.

Code changes make sense to me and I haven't noticed any issues when I clicked through it briefly, however it's a significant change in the core logic of the component and I'd like to be more confident we don't introduce unexpected regressions. Historically, any updates around focus rings tend to produce bugs in situations we perhaps didn't think of before.

Could you please test textbox keyboard and mouse navigation and outlines thoroughly? Also, if you find any issues, it'd be helpful to check whether it was present on develop before so we can be sure that this change is indeed the real cause. In this case, it will really help with debugging. Thank you.

@github-actions github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) labels Sep 22, 2023
@MisRob MisRob changed the base branch from release-v0.16.x to develop September 22, 2023 17:38
@MisRob MisRob changed the title Fix checkbox being unexpectedly focused after a first page load [DO NOT MERGE] Fix checkbox being unexpectedly focused after a first page load Sep 22, 2023
@MisRob MisRob added the TAG: a11y Affecting accessibility label Sep 22, 2023
@MisRob MisRob changed the title [DO NOT MERGE] Fix checkbox being unexpectedly focused after a first page load [DO NOT MERGE] Fix checkbox being unexpectedly focused after the first page load Sep 22, 2023
@pcenov
Copy link
Member

pcenov commented Sep 25, 2023

@MisRob seems that there is a problem with the build?

@MisRob
Copy link
Member Author

MisRob commented Sep 25, 2023

@pcenov Ah it got stuck somehow. Thank you, I will let you know when it's resolved.

@github-actions github-actions bot removed DEV: backend Python, databases, networking, filesystem... APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) labels Sep 25, 2023
@MisRob
Copy link
Member Author

MisRob commented Sep 27, 2023

@pcenov Builds should be now ready

@pcenov
Copy link
Member

pcenov commented Sep 28, 2023

Hi @MisRob I confirm that the issue described in #9077 is now fixed and there are no regressions caused by this change elsewhere.

@MisRob
Copy link
Member Author

MisRob commented Sep 28, 2023

Great, thanks @pcenov

@MisRob MisRob changed the title [DO NOT MERGE] Fix checkbox being unexpectedly focused after the first page load [DO NOT MERGE] Fix textbox being unexpectedly focused after the first page load Sep 28, 2023
@MisRob
Copy link
Member Author

MisRob commented Sep 28, 2023

The force push only replaced the fork reference to the latest KDS

@MisRob
Copy link
Member Author

MisRob commented Sep 29, 2023

I think tests were passing when I referenced the fork and the failing tests occurred after referencing KDS. Makes me think that this installs another KDS update that we forgot to install and that broke tests. I will investigate.

@MisRob
Copy link
Member Author

MisRob commented Oct 4, 2023

Superseded by #11315

@MisRob MisRob closed this Oct 4, 2023
@MisRob MisRob deleted the checkbox-focus branch February 29, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SIZE: very small TAG: a11y Affecting accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Two inputs are focused at the same time
2 participants