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

Focus state being reset by browser in pagination component #1040

Closed
mr-gabe49 opened this issue Feb 2, 2022 · 6 comments · Fixed by #1212
Closed

Focus state being reset by browser in pagination component #1040

mr-gabe49 opened this issue Feb 2, 2022 · 6 comments · Fixed by #1212
Assignees
Labels
good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. help wanted Community contributors can implement this type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory

Comments

@mr-gabe49
Copy link
Member

mr-gabe49 commented Feb 2, 2022

Pagination component improvement:

Originally posted by @pavish in #963 (comment)

Issue:

  1. Ellipsis is shown and user clicks on it (i.e the arrow).
  2. The page number changes but ellipsis/arrow is now not present because it is not required. Eg., Ellipsis is present when active page is 7. Clicking on it changes active page to 2, and ellipsis is no longer present.
  3. Since the last focused element is no longer present on the dom, the focus state is reset by the browser.

We should be resolving this by manually setting focus on the current active page when ellipsis/arrow disappears. In the above example, when we click on the ellipsis and page number is changed to 2, we should be doing element.focus() to focus the button with page number 2. We can add a data-page attribute to help query the element.

@mr-gabe49 mr-gabe49 added good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. ready Ready for implementation type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory labels Feb 2, 2022
@mr-gabe49 mr-gabe49 added this to the [10.1] 2022-03 improvements milestone Feb 2, 2022
@pavish pavish changed the title Focus state being reset by browser Focus state being reset by browser in pagination component Feb 3, 2022
@priyang12
Copy link
Contributor

@pavish @seancolsen hi, can i work on this issue?

@kgodey kgodey added help wanted Community contributors can implement this status: started and removed ready Ready for implementation labels Mar 19, 2022
@kgodey
Copy link
Contributor

kgodey commented Mar 19, 2022

Go ahead @priyang12, thanks!

@priyang12
Copy link
Contributor

@kgodey hi i think i have resolved this issue here is a

Screen.Recording.2022-03-22.at.1.58.00.PM.mov

clip of it . I have changed the color to red for better check but i have removed it later.

@seancolsen
Copy link
Contributor

seancolsen commented Mar 22, 2022

@priyang12

i think i have resolved this issue

It sounds like you're making some progress. That's great! Have you opened a PR with your work? I don't see one. Make sure to link your PR to this ticket by placing the text Fixes #1040 in the top of the description. Your above screencap would be great to include in the PR description too. We can use the PR to continue discussing your fix.

@priyang12
Copy link
Contributor

priyang12 commented Mar 22, 2022

@seancolsen I have one PR 1200 open so I can't make another one. should I make a new branch and make a pull request?

@pavish
Copy link
Member

pavish commented Mar 22, 2022

@priyang12 Yes, please create new branches for each of the issue you're working on and open separate PRs.

pavish added a commit that referenced this issue Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. help wanted Community contributors can implement this type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants