kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

Two inputs are focused at the same time

Open sairina opened this issue 3 years ago • 4 comments

Observed behavior

In the user profile, when a user navigates to the "Edit" button with a keyboard and enters it, they are immediately greeted with the focus on both the "Full name" and "User name" inputs.

2022-02-02 09 13 19

Errors and logs

No visible errors

Expected behavior

The focus should be on the first input, and as the user navigates through with a keyboard, they should be able to move back and forth.

User-facing consequences

Confusing for accessibility, as you can type into only the first, focused element (the full name text box) even when both are highlighted.

Steps to reproduce

  1. Sign in
  2. Open the side panel on the left-hand side
  3. Use the keyboard to navigate to "Profile"
  4. Use the keyboard to navigate to the "Edit" button
  5. Notice that you immediately see the focus on both of the text boxes.

Notes: The component where this issue lives is the ProfileEditPage, and the FullNameTextbox and UsernameTextbox are the affected components. The code for KTextbox can be found here, and the documentation for it can be found here.

Context

Kolibri 0.15 Chrome Mac

sairina avatar Feb 02 '22 17:02 sairina

Hey, @sairina I would like to contribute to this issue. Thanks :)

iprasandeep avatar Feb 06 '22 22:02 iprasandeep

Hi @prsndp, would love your help. Thank you so much for offering! I've added some more details in the issue that should help you figure out where you want to start investigating. Feel free to reach out if you have additional questions.

You may want to also look at our design system repo, as the components that are affected are built on KTextbox.

sairina avatar Feb 07 '22 05:02 sairina

@sairina Thanks:) Yeah, Sure!

iprasandeep avatar Feb 07 '22 12:02 iprasandeep

Hi @prsndp! Just checking in if this issue is resolved or not. I'm a new contributor and I'm looking for a good first issue to work on.

zhihanC avatar Apr 11 '22 03:04 zhihanC

Hi @prsndp @zhihanC @marcellamaki Is this issue resolved? Can I take it up?

kshitijsaksena avatar Jan 08 '23 10:01 kshitijsaksena

Hi @kshitijsaksena - I don't believe this issue has been resolved from anyone specifically working on it. However, we have made some other adjustments to our focus outline over the last months, and it may have been resolved incidentally.

You are welcome to try replicating the issue and proceed from there! If it is no longer replicable, please just add a screenshot in the comment, and we can close it out and find an alternative issue for you to work on. If it is replicable, we would love your contribution! Please ask here in a comment if you have any questions and tag me. Thank you!

marcellamaki avatar Jan 09 '23 13:01 marcellamaki

Hi there @marcellamaki and @sairina I was trying to reproduce this error to see if I could take on this issue if it is still available. Can you state here what browser extension / tool / scripts did you use to get that border around the highlighted fields?

Akila-I avatar Feb 24 '23 12:02 Akila-I

Hello @Akila-I, this logic comes from Kolibri Design System. Here are some related places in code:

  • https://github.com/learningequality/kolibri-design-system/blob/develop/lib/KThemePlugin.js#L47-L54
  • https://github.com/learningequality/kolibri-design-system/blob/develop/lib/styles/trackInputModality.js

MisRob avatar Mar 07 '23 07:03 MisRob

@MisRob, thank you so much for for following up!

@Akila-I - in order for the focus ring to appear, you should use keyboard navigation (usually the tab key - sometimes arrow keys as well) rather than a mouse for moving around the page. Depending on your OS, you may need to update your system settings to ensure the focus outline is consistently displayed, as it is not the default on Mac OS.

marcellamaki avatar Mar 07 '23 14:03 marcellamaki

Thanks for the tip @marcellamaki. I was able to reproduce the behaviour described in the issue. And thanks @MisRob for the pointers. I will have a look on this issue as well when I got time.

Akila-I avatar Mar 07 '23 18:03 Akila-I

Hey, I am interested in working on this issue. Could you please assign it to me @MisRob

Pursottam6003 avatar Mar 31 '23 12:03 Pursottam6003

Hello @Pursottam6003, thank you for your interest. As you volunteered for more issues and already have some of them assigned, I'm leaving this unassigned for now in case someone else wanted to tackle it as you're working on other issues. When your plate is empty, if this issue is still available, please remind yourself here and we can assign it to you.

MisRob avatar Apr 05 '23 10:04 MisRob

Hi, I am interested in working on this issue. Could you please assign it to me @MisRob

pratiksarkar55 avatar Apr 10 '23 08:04 pratiksarkar55

Hi @pratiksarkar55, thank you for your interest. You're welcome to take this issue on.

If you haven't done so already, please have a look at our developer documentation.

In addition to that, it may be helpful to read through the comments above as there are some pointers to relevant places and repositories. I can't say for sure, however, it may be the case that the problem comes from the Kolibri Design System repository which too is mentioned there.

This issue is to be resolved on the develop branch so please open a pull request into develop. If you have any problems running Kolibri locally or have questions, you can use the comments section on this page. As part of your pull request, you're welcome to add yourself to the list of contributors in AUTHORS.md file.

MisRob avatar Apr 10 '23 10:04 MisRob

Sure.Thank you for your response.

On Mon, 10 Apr, 2023, 16:20 Michaela Robosova, @.***> wrote:

Hi @pratiksarkar55 https://github.com/pratiksarkar55, thank you for your interest. You're welcome to take this issue on.

If you haven't done so already, please have a look at our developer documentation https://kolibri-dev.readthedocs.io/en/develop/getting_started.html.

In addition to that, it may be helpful to read through the comments above as there are some pointers to relevant places and repositories. I can't say for sure, however, it may be the case that the problem comes from the Kolibri Design System repository which too is mentioned there.

This issue is to be resolved on the develop branch so please open a pull request into develop. If you have any problems running Kolibri locally or have questions, you can use the comments section on this page. As part of your pull request, you're welcome to add yourself to the list of contributors in AUTHORS.md file.

— Reply to this email directly, view it on GitHub https://github.com/learningequality/kolibri/issues/9077#issuecomment-1501671199, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGZH5T4V5QWAG7COIHQUZPLXAPQYFANCNFSM5NMXOLPQ . You are receiving this because you were assigned.Message ID: @.***>

pratiksarkar55 avatar Apr 10 '23 11:04 pratiksarkar55

Hello @MisRob i am working on this issue currently. Could you assign it to me?

thesujai avatar Sep 12 '23 03:09 thesujai

I understand what changes are to be done in kolibri-design-system/lib/KTextbox. How do i verify the changes are affecting because when i edit the file in node_modules/kolibri-design-system/lib/KTextbox it doesn't work, how is it working?

thesujai avatar Sep 12 '23 04:09 thesujai

Hi @thesujai. thank you. I'm assigning you.

when i edit the file in node_modules/kolibri-design-system/lib/KTextbox it doesn't work

I think this might be a problem we described in this issue https://github.com/learningequality/kolibri/issues/10808. If it seems to be the case, can you try running Kolibri with yarn run build and then yarn run python-devserver and see if your updates propagate? Note that hot reload won't work, and you will need to run these two commands after each update which may be slow and cumbersome, but I am not aware of any other workaround at this point.

If you verify that your changes work, this is the repository of Kolibri Design System (KDS) https://github.com/learningequality/kolibri-design-system which contains KTextbox code. It's okay modify Kolibri's node_modules directly to verify your changes. After that, those updates will need to happen in the KDS repo. You can find more information on how to contribute to KDS here https://github.com/learningequality/kolibri-design-system/blob/cabbfbd4bbdc5d2f192cc276a61654cc4d2036ee/CONTRIBUTING.md. Let me know if you had any questions.

MisRob avatar Sep 12 '23 14:09 MisRob

@MisRob Thank you for the help. When we navigate to pages using pages using keyboard, no page will have the coreOutline with any component by default, it is only the ProfileEditPage with this problem. When we refresh the edit page and then press Tab we observe that the navigation works fine, the problem is only when then the page loads initially. The most optimal solution now can be that when the ProfileEditPage opens there should be no coreOutline, it should have to do with the routing right?

thesujai avatar Sep 12 '23 15:09 thesujai

@thesujai Yes, it seems that the next step would be to find out why the ProfileEditPage behaves differently. I am not sure if routing will be the cause, but you can check it out for sure.

I would also recommend debugging two places referenced in one of the comments above https://github.com/learningequality/kolibri/issues/9077#issuecomment-1457703585. It includes the script the makes some major decisions in regards to focus rings.

MisRob avatar Sep 13 '23 11:09 MisRob

@MisRob Thank you for the help The problem is arising because of the Design of KTextbox which is supposed to have coreOutline property wrt to changedOrFocused(https://github.com/learningequality/kolibri-design-system/blob/develop/lib/KTextbox/index.vue#L16)

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.

this line(https://github.com/learningequality/kolibri-design-system/blob/develop/lib/KTextbox/index.vue#L170 ) is causing this behavior, I see no reason to set changedOrFocused for having coreOutline when the value of textbox is updated. I can see removing that line does solves this problem

As this problem is related with kolibri-design-system, where do I create a pr?

thesujai avatar Sep 13 '23 12:09 thesujai

@thesujai I can't say off the top of my head, but seeing the code, it seems there might be some use-cases for which tracking 'changed' state is needed. I will have a look in more detail later on. In any case, we will need indeed make some updates to that logic to work well for the situation you describe. Thanks for debugging!

As this problem is related with kolibri-design-system, where do I create a pr?

Please see https://github.com/learningequality/kolibri/issues/9077#issuecomment-1715813878 where kolibri-design-system repository is linked. That would be a place to open a PR.

If you verify that your changes work, this is the repository of Kolibri Design System (KDS) https://github.com/learningequality/kolibri-design-system which contains KTextbox code. It's okay modify Kolibri's node_modules directly to verify your changes. After that, those updates will need to happen in the KDS repo. You can find more information on how to contribute to KDS here https://github.com/learningequality/kolibri-design-system/blob/cabbfbd4bbdc5d2f192cc276a61654cc4d2036ee/CONTRIBUTING.md. Let me know if you had any questions.

MisRob avatar Sep 15 '23 06:09 MisRob

I have created a pr for this, please review it

thesujai avatar Sep 15 '23 11:09 thesujai

Thanks @thesujai, we will have a look

MisRob avatar Sep 18 '23 12:09 MisRob

Fixed by https://github.com/learningequality/kolibri-design-system/pull/449 and merged to Kolibri by https://github.com/learningequality/kolibri/pull/11315

MisRob avatar Oct 04 '23 11:10 MisRob