slider: Not accessible

@byCedric Great work! One question (I can open a separate issue on this if needed): onMouseDown() in createSlider.jsx effectively swallows the mouse event causing the handle not to be focusable by clicking on it. I’m unsure what the correct way of solving this is, but I validated it by simply remove that function and the handles become click-focusable

So basically its now possible by creating a new handler and provide some event handling? Personally I would love if this was an out-of-the-box feature. Btw, great job on the aria attributes already!

I can try to write something for this and create a PR if that’s appreciated. If it is, maybe its best to discuss the behaviours since I encountered some differences at e.g. OAA Accessibility and Polymer.

What are others doing?

Basically the only thing what others are doing is making the handler focusable and provide some keyboard events.

key	Polymer	OAA Accessibility
left	value - 1 step	same
right	value + 1 step	same
up	value + 1 step	value - 1 step
down	value - 1 step	value + 1 step
home	value = minimum	same
end	value = maximum	same
page-up	value + 1 step	value - 1 “jump step”
page-down	value - 1 step	value + 1 “jump step”

These are things that happens with left to right languages, for right to left languages everything is mirrored.

I dont know if it’s possible to create an inverted range. E.g. max to min instead of min to max. But if this is possible I think it should be handled like right to left languages.

• Value is the current value of the handle.
• Step is every value from the range you can select.
• Jump step is a custom definable value, at OAA Accessibility it’s 2 * step.

Some random thoughts

Both Polymer and the OAA Accessibility looks like great organisations that does a great job. They probably have thought about it for a while and still there are some differences. Lets try to reason and explain them.

Since both organisations have the same behaviour on the left and right keys, I think its best to just copy that too.

For the up and down behaviours I tend to follow the Polymer approach. Mainly because you are “upping” or “downing” the current value. E.g. if I have 8 as current, and I “up” it with a step of 1, I expect it to be 9 and not 7.

OAA Accessibility has a point about the page up and page down behaviours. I mean, left/down and up/right only go by 1 step. For small sliders this is fine, but for large ones not. Also if an user actually uses the page up/page down key, they probably expect something more then just a mimick of left and right.

---

So what do you guys think? Or do you guys dont want to support this feature or think it should not be implemented in this repository?

I’d be interested in helping to make this component accessible. Currently, the API does not allow for passing callbacks to custom Handle components (from what I can tell) meaning that CustomHandle implementations can’t do much aside from modify style.

It would be great if we could provide a handleChange prop to the handle, allowing a developer to create a function to handle things like keyboard arrow up and keyboard arrow down when the user is focused on the button element.

Something like this is what I have in mind, would this be possible?

class Handle extends React.Component { // eslint-disable-line react/prefer-stateless-function
  constructor() {
    super();
    this.button = null;
  }

  componentDidMount() {
    if (canUseDOM && canUseEventListeners) {
      document.addEventListener('keydown', this.handleKeydown);
    }
  }

  componentWillUnmount() {
    if (canUseDOM && canUseEventListeners) {
      document.removeEventListener('keydown', this.handleKeydown);
    }
  }

  handleKeydown = ({ keyCode } : { keyCode: number }) => {
    if (document.activeElement === this.button && keyCode === 38) {
      // Keyboard-Arrow-Up Increment
      this.props.handleChange(this.props.index, 'increment');
    } else if (document.activeElement === this.button && keyCode === 40) {
      // Keyboard-Arrow-Down Decrement
      this.props.handleChange(this.props.index, 'decrement');
    }
  }

  props: HandleProps;

  render() {
    const { className, offset } = this.props;

    return (
      <button
        className={classNames(className, styles.button)}
        style={{ left: `${offset}%` }}
        ref={(button) => { this.button = button; }}
      />
    );
  }
}

cc @benjycui

@byCedric Welcome to open another pr to help us make it better 😄

Thanks for @byCedric great job! https://github.com/react-component/slider/pull/282

We have released rc-slider@8.3.0.

Ok, so I have been busy and I got further than I hoped to be. In this pull request I did the following things.

What I did

Added basic keyboard interaction

I did this by adding a tabindex="0" to the handle. With that the browser allows to focus non-focusable elements. So this change is actually perfect to implement accessibility without rewriting everything.

Also I added basic keyboard event binding such as onFocus, onBlur and onKeyDown. Both the focus and blur are defined to trigger onStart and onEnd events. I figured it would calculate some stuff that is necessary. I’m not sure if thats 100% right, so correct me if I’m wrong.

Added the onKeyboard handler to both Slider and Range

At first I thought I could fit all calculations within the createSlider method itself. Well, when I took a look at the Range I saw that the “current value” was handled differently. Therefore I figured both Slider and Range should have different calculations when the keyboard event was invoked.

Added keyboard value mutators

I added the concept of “keyboard value mutator” after I got stuck in defining the outcome of all key events. With this you will get a “value mutating method” that accepts the current, or old, value and the props of the slider component. With this it should be capable of all, possible future, keyboard events.

Some issues

As mentioned in the pull request this is a WIP. I currently encountered some issues of which I think its best to discuss them.

When no step is defined (step=null) the mutators that uses step won’t work.

I’m not sure what the meaning of step=null is and/or should be. Unfortunately this scenario breaks all keyboard behaviour, except for the home and end keys. This is because they don’t use step to calculate the next value. We could however provide a simple “best guess” to work around this issue.

When I implement the onKeyboard for the Range, everything works until the handles “cross”.

Well, I guess this is best explained with a short video.

What happens here is that when I “cross” with the other handle, the focus is still at the old one. But it looks like I’m actually changing the value of the other handle… I’m not sure if this is a bug from the keyboard events, or that this is a bug in general. It happens with the mouse pointer too… I think it’s best to open another issue for this, or do you think differently?

I also tested it with the multi range and that seems to work perfectly!

---

So what do you think @benjycui?