Better user interface for screen readers and keyboard navigation#2946
Better user interface for screen readers and keyboard navigation#2946richvdh merged 1 commit intoelement-hq:developfrom
Conversation
|
can you rebase this on latest develop so it doesn't include a load of unrelated changes? |
b2d1505 to
410e685
Compare
|
Requires matrix-org/matrix-react-sdk#616, ftr |
richvdh
left a comment
There was a problem hiding this comment.
a couple of nits it would be nice to fix
| buttonGroup = | ||
| <div className="mx_RightPanel_headerButtonGroup"> | ||
| <div className="mx_RightPanel_headerButton" title="Members" onClick={ this.onMemberListButtonClick }> | ||
| <div className="mx_RightPanel_headerButton"> |
There was a problem hiding this comment.
are the divs still needed here?
There was a problem hiding this comment.
Didn't want to touch it just in case.
There was a problem hiding this comment.
Seems like it'd be fine if the class were to be moved to the button element.
|
|
||
| _clearSearch: function() { | ||
| this.refs.search.value = ""; | ||
| this.onChange(); |
There was a problem hiding this comment.
this should happen automatically?
There was a problem hiding this comment.
You're kinda right, for reasons that are ... non-obvious. In case you're interested, and for my own reference:
Essentially the onChange callback is a React fabrication. Native-DOM <input> elements have an onchange attribute, but it is only called when the element loses focus.
React actually listens for the input event, and calls the onChange callback when it sees one. Apparently the DOM does not fire an input event when the value is changed from js - which is probably lucky otherwise React would have to jump through hoops to avoid getting stuck in a loop.
What we ought to do here is stick with the react side, and just make the change through react:
this.setState({ searchTerm: "" });
That will trigger a re-render, which will update the DOM.
See also: https://facebook.github.io/react/docs/forms.html#controlled-components.
|
@JaniM hang on - i'm confused. i thought this had already landed? or did we only merge the react-sdk half of it? |
Changed most button-like divs to actually behave as such (via the new AccessibleButton element). Also made filtering rooms more keyboard friendly.
Signed-off-by: Jani Mustonen janijohannes@kapsi.fi