-
Notifications
You must be signed in to change notification settings - Fork 290
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
Merge master to v1.0-beta #475
Conversation
… (Helps #440) This seems to have helped the screen reader identify where it is when focus is within the table. When using Narrator in Edge or Accessibility tools with Firefox, the screen reader says the roles correctly, reads the column with the cell, can tell which column it is in, and which row it is in.
@@ -22,6 +22,7 @@ import PropTypes from 'prop-types'; | |||
import ReactTouchHandler from 'ReactTouchHandler'; | |||
import ReactWheelHandler from 'ReactWheelHandler'; | |||
import Scrollbar from 'Scrollbar'; | |||
import ariaAttributesSelector from 'ariaAttributes'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this to a selector. Will be easier to manage in the near future.
this._touchHandler.onTouchMove, | ||
{ passive: false } | ||
); | ||
if (this.props.touchScrollEnabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only add the touch event handler if the prop is on
onTouchStart={this._touchHandler.onTouchStart} | ||
onTouchEnd={this._touchHandler.onTouchEnd} | ||
onTouchCancel={this._touchHandler.onTouchCancel} | ||
onTouchStart={touchScrollEnabled ? this._touchHandler.onTouchStart : null} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't register touch event handlers unless prop is on
@@ -512,11 +512,11 @@ class Scrollbar extends React.PureComponent { | |||
} | |||
|
|||
getTouchX = (/*object*/ e) => { | |||
return Math.round(e.targetTouches[0].pageX - e.target.getBoundingClientRect().x); | |||
return Math.round(e.targetTouches[0].clientX - e.target.getBoundingClientRect().x); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes touch positioning of the scrollbar thumb
* ariaRowIndexOffset: number | ||
* }} | ||
*/ | ||
function calculateAriaAttributes(rowsCount, useGroupHeader, useFooter) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Tanner-MS , can you review this function? I just copied your PR for master, and renamed/rearranged a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Description, Motivation and Context
Merges latest master to v1.0-beta to prepare for 1.0-beta production release.
Improvements/bug fixes that gets merged from master:
How Has This Been Tested?
Tested on our examples and verified the role attributes and touch scrolling.
Types of changes
Checklist: