Skip to content

Helping with dual range input props#2

Merged
ffknob merged 1 commit intoffknob:accesibility-EuiDualRange-aria-labelsfrom
cchaos:accesibility-EuiDualRange-aria-labels/cchaos
Jan 8, 2020
Merged

Helping with dual range input props#2
ffknob merged 1 commit intoffknob:accesibility-EuiDualRange-aria-labelsfrom
cchaos:accesibility-EuiDualRange-aria-labels/cchaos

Conversation

@cchaos
Copy link
Copy Markdown

@cchaos cchaos commented Jan 8, 2020

I'll comment in the diff the changes I've made.

## [`master`](https://github.com/elastic/eui/tree/master)

- Added possibility to set `aria-label` for both `min` and `max` inputs of `EuiDualRange` ([#2738](https://github.com/elastic/eui/pull/2738))
- Added `minInputProps` and `maxInputProps` to supply more props to the inputs of `EuiDualRange` ([#2738](https://github.com/elastic/eui/pull/2738))
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just made this more generic since it's no longer just about aria-label

// Overridable props
aria-describedby={this.props['aria-describedby']}
aria-label={this.props['aria-label']}
{...minInputProps}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the order that I was trying to mention before. All the props below this line are mandator and therefore cannot be allowed to be overridden by ...inputProps. The only way to ensure this is that ...inputProps comes before the mandator props.

expect(component).toMatchSnapshot();
});

test('should be set only for the min input', () => {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the tests below were still too specific for aria-label, and really just ensuring that the input props get passed down, doesn't error out, is all that's needed here so I simplified the version above and removed these and below.

@ffknob ffknob merged commit 25a7e5a into ffknob:accesibility-EuiDualRange-aria-labels Jan 8, 2020
@cchaos cchaos deleted the accesibility-EuiDualRange-aria-labels/cchaos branch January 8, 2020 22:18
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 this pull request may close these issues.

2 participants