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

Stuck in the grid using keyboard navigation when the cells are not editable #518

Closed
bblwq opened this issue Jun 30, 2020 · 7 comments · Fixed by #976
Closed

Stuck in the grid using keyboard navigation when the cells are not editable #518

bblwq opened this issue Jun 30, 2020 · 7 comments · Fixed by #976

Comments

@bblwq
Copy link

bblwq commented Jun 30, 2020

I have an application that implements SlickGrid and we only allow visitors to view the data, just like this example: http://6pac.github.io/SlickGrid/examples/example1-simple.html.

When we did our accessibility testing, we found one could be stuck in the grid using keyboard navigation only because they're just way too many cells. This issue will create even more confusion for screen reader users because the screen reader will not read out the cell value but simply keep silent from cell to cell.

After some research, I figured the best solution is to bypass the table

using tabindex="-1" (for example) or so. Please comment below if you have any better ideas or have the issue addressed in other branches.

@ghiscoding
Copy link
Collaborator

I don't fully understand your problem but just want to mention that SlickGrid was written over 10 years ago (before the first iPhone) so it wasn't created with mobile in mind and most of the events are mouse or keyboard events which might not always work with a mobile phone. If you find where the problem occurs then please create a PR (Pull Request)

@ghiscoding
Copy link
Collaborator

ghiscoding commented Jul 31, 2020

Hey I was googling for something completely different about SlickGrid and felt on this PR that was closed (never merged I think) on the original SlickGrid fork which does mention screen reader, so perhaps it might help with this issue.

There's this comment that the user wrote in his PR which mentions screen reader

SlickGrid doesn't work for people who use screen readers right now. When navigating the grid with the keyboard, screen readers aren't aware that anything is happening. This change introduces a few simple changes that make slick grid much more usable for people that have to use a screen reader:

@sardell
Copy link

sardell commented Sep 29, 2020

@ghiscoding I would love to see those a11y changes land in this project. Would be glad to help contribute in any way I can.

@ghiscoding
Copy link
Collaborator

@sardell if you find code to fix this issue, we are totally open for any Pull Requests. Perhaps you could clone the 6pac/SlickGrid on your local and try to merge the code from this old PR that I referenced in previous comment. If you that code works, then please do a PR and we'll review it and get it merged.

@6pac
Copy link
Owner

6pac commented Sep 29, 2020

@sardell
Copy link

sardell commented Oct 2, 2020

@ghiscoding I attempted to merge the code from this PR as you suggested. However, the PR is so old that the merge conflict was essentially two large diffs. Luckily, the PR is small enough that I was able to find where to apply the changes in the current file's state.

There are two issues I ran into:

  1. The most important change in the PR cannot be applied because the if condition is commented out in the current code.
// this optimisation causes trouble - MLeibman #329
      //if (activeCellChanged) {
      if (!suppressActiveCellChangedEvent) { trigger(self.onActiveCellChanged, getActiveCell()); }
      //}
  1. Although the work contained in the PR sets focus to the current active cell, we still need to implement a focus strategy using a roving tabindex. I'm guessing this is what @bblwq means with their issue, but they'll need to confirm. However, this might present an issue for SlickGrid: One benefit of using roving tabindex...is that the user agent will scroll the newly focused element into view.

@ghiscoding
Copy link
Collaborator

@sardell I think that @6pac (Ben) knows more about this commented out code since I think he his the person who commented out this piece of code. I never heard of the word roving, I had to google it (I'm bilingual but English is my 2nd language so) lol

ghiscoding pushed a commit that referenced this issue Jan 16, 2024
- this PR replicates an old SlickGrid PR (except the focus which can cause other issues) mleibman/SlickGrid#616
ghiscoding added a commit that referenced this issue Jan 16, 2024
#976)

- this PR replicates an old SlickGrid PR (except the focus which can cause other issues) mleibman/SlickGrid#616
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants