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

Accessability #11

Open
Tal500 opened this issue Jun 20, 2022 · 4 comments · May be fixed by #47
Open

Accessability #11

Tal500 opened this issue Jun 20, 2022 · 4 comments · May be fixed by #47
Assignees
Labels
enhancement New feature or request

Comments

@Tal500
Copy link
Collaborator

Tal500 commented Jun 20, 2022

Following #9, we need to have accessibility for the splitter, by ARIA attributes and by keyboard interaction.

  1. MDN on separator role
  2. W3 on windows splitter.
  3. See also the list of proposed examples in this issue. I'd personally think that this one is the clearest.

There are two types of splitters - non-focusable(i.e. static) and focusable(interactive). Read source 1 for telling which aria attributes should be on each on.
I believe that a splitter in this library is consider to be non-focusable(i.e. static) if and only if minSize===maxSize, as you can see the example in this library docs.

Focusable (interactive) splitter in this library

  • Should have the HTML attribute tabindex = 0. Notice that non-focusable(i.e. static) splitters should not be focusable at all!
  • I believe that default style when it's focused should be like in hover.
  • I believe we should implement all the keyboard binding the same as described in Source 2 (including the optional ones). The keys should be as what described there by default, but allow the user to change the key binding he prefer. Notice that if we're in RTL, the right arrow key should point to the previous pane!
  • Notice in Source 2 the key binding of F6(cycle between panes) is a really cool one in my opinion. I think that each pane should have the HTML attribute tabindex = -1(meaning focusable, but only by script), and pressing F6 will detect which pane has the current focus, and will cycle through the panes and set the focus on the next one. I believe, although not written by the official standards, the if the user press Shift + F6, then it should cycle backwards, as the Shift key invert the operation.
  • Continuing the previous bullet on F6, I believe that a special care should be for nested panes. I think the correct way to deal with this issue is to cycle only through the leaf panes(meaning, on on panes which don't have a nested SplitPanes component inside of them).

I think we need to be careful and clever for dealing with Svelte context API for passing sizes data from SplitPanes to Pane components.

I have an idea how to implement it, just need a green light to do so.

@orefalo orefalo added the enhancement New feature or request label Jun 21, 2022
@Tal500
Copy link
Collaborator Author

Tal500 commented Jun 21, 2022

Tnx, will try to work on this next week.

Forgot to mention:

  • Need to style (by default) the focusing of the pane while pressing F6

Tal500 added a commit that referenced this issue Aug 2, 2022
@Tal500 Tal500 linked a pull request Dec 29, 2022 that will close this issue
@FernieRealEstate
Copy link

Hi, I also get this annoying clutter in Terminal, which I can't figure out how to "ignore":
vite v4.1.2 building SSR bundle for production...
transforming (107) node_modules/.pnpm/@skeletonlabs[email protected]/node_modules/@skeletonlabs/skeleton/themes/theme-modern.css7:22:17 AM [vite-plugin-svelte] /node_modules/.pnpm/[email protected][email protected]/node_modules/svelte-splitpanes/Pane.svelte:117:1 A11y: visible, non-interactive elements with an on:click event must be accompanied by an on:keydown, on:keyup, or on:keypress event.
115:
116:
117: <div
^
118: class={splitpanes__pane ${clazz || ''}}
119: bind:this={element}

@Tal500
Copy link
Collaborator Author

Tal500 commented Feb 20, 2023

Hi, I also get this annoying clutter in Terminal, which I can't figure out how to "ignore": vite v4.1.2 building SSR bundle for production... transforming (107) node_modules/.pnpm/@skeletonlabs[email protected]/node_modules/@skeletonlabs/skeleton/themes/theme-modern.css7:22:17 AM [vite-plugin-svelte] /node_modules/.pnpm/[email protected][email protected]/node_modules/svelte-splitpanes/Pane.svelte:117:1 A11y: visible, non-interactive elements with an on:click event must be accompanied by an on:keydown, on:keyup, or on:keypress event. 115: 116: 117: <div ^ 118: class={splitpanes__pane ${clazz || ''}} 119: bind:this={element}

I agree that it's annoying. Since we are aware of this warning and this is WIP, I guess we shall use the "svelte a11y ignore" tags:
https://svelte.dev/docs#accessibility-warnings

We should comment in the code that the issues are know and reported on this issue, i.e. add next to the tags:

<!-- this a11y are known, and will be taken care of as part of the a11y feature issue in #11 -->

Tal500 added a commit that referenced this issue Feb 26, 2023
… warnings

We explain in the code that this will be fixed in the future as part of #11.

Even though in general we should keep the warning in the console until they're fixed,
the library users should not be annoyed in their own builds.

Refs: #11 (comment)
@Tal500
Copy link
Collaborator Author

Tal500 commented Feb 26, 2023

Hi, I also get this annoying clutter in Terminal, which I can't figure out how to "ignore": vite v4.1.2 building SSR bundle for production... transforming (107) node_modules/.pnpm/@skeletonlabs[email protected]/node_modules/@skeletonlabs/skeleton/themes/theme-modern.css7:22:17 AM [vite-plugin-svelte] /node_modules/.pnpm/[email protected][email protected]/node_modules/svelte-splitpanes/Pane.svelte:117:1 A11y: visible, non-interactive elements with an on:click event must be accompanied by an on:keydown, on:keyup, or on:keypress event. 115: 116: 117: <div ^ 118: class={splitpanes__pane ${clazz || ''}} 119: bind:this={element}

This comment is fixed now by 7d9b872.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants