-
-
Notifications
You must be signed in to change notification settings - Fork 332
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
Frontend component: Pagination improvements #292
Comments
@pavish I think you could repurpose this issue to cover virtual scrolling. Please update the issue. |
@kgodey We have a reusable pagination component, which is part of the component library. We can have a separate issue for virtual scrolling. |
@pavish Since the infinite scrolling PR is already in review, I don't think there's a need to create an issue for it. If there's additional accessibility, documentation, or testing concerns, I think you're a better person to make issues for those than me. |
I am trying to setup this repository in my local. I am using Windows OS. I have installed docker. I am facing difficulty in the following step. |
@agrawalanshul053 These commands are to copy files in a posix-like environment. In windows, you can copy the Alternatively, you can use your WSL (Windows subsystem for Linux) shell, and use posix-like commands. |
@agrawalanshul053 This issue is for the frontend component, I have moved this to a new issue here #830. Please use it for further discussion |
@pavish I'll take the documentation |
Sure @mr-gabe49. Thanks! |
@pavish @seancolsen I would like some guidance on the behavior we are looking for on keyboard accessibility. I have seen different behaviors through the web:
Let me know your thoughts. Thanks! |
@mr-gabe49 I prefer Left and Right arrow changing focused page and clicking enter would change current page. I would not use up and down since browsers by default use them for scrolling the webpage.
I think this would be more confusing and we can avoid this for now.
We could disable the arrows instead of removing them entirely. Removing the arrow would change a shift in layout causing a sudden jerk. I suggest having a look at ant.design pagination component as a reference. Having said that, we could always do our own research and improve upon this. At the moment, I feel it would be better to follow accessibility patterns well established by other popular frameworks. |
@pavish Sounds good, thanks! |
The pagination component is very similar to a navigation header, in terms of markup and usage. There are a few things we need to handle to ensure proper accessibility:
References: |
The following still needs to be handled as part of this issue:
We already have tests for paginationUtils as part of #925. We require tests for the DOM rendering aspect of the component. |
I'm closing this because it's been open for almost a year. All that's left here is to add tests. While additional test coverage would be nice for this component, I don't see it being a priority. We have plenty of front end code without test coverage, and this code doesn't strike me as any more important than other areas of our codebase where we could seek to improve test coverage. |
The followed needs to be handled:
PRs that are related to this issue:
The text was updated successfully, but these errors were encountered: