Skip to content

Conversation

gazab
Copy link
Contributor

@gazab gazab commented Apr 2, 2019

What does it do?

Adds some ARIA roles and attributes according to:

Does not add label to input element because #225 already solves that.

Fixes # (issue)

Partly addresses #216

Type of change

  • New feature (non-breaking change which adds functionality)

@gazab gazab changed the title Accessibility: Add ARIA roles and attributes feat: Add ARIA roles and attributes Apr 2, 2019
@coveralls
Copy link

coveralls commented Apr 2, 2019

Pull Request Test Coverage Report for Build 1110

  • 37 of 38 (97.37%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.09%) to 94.486%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/trigger/index.js 14 15 93.33%
Totals Coverage Status
Change from base Build 1100: -0.09%
Covered Lines: 564
Relevant Lines: 579

💛 - Coveralls

@mrchief
Copy link
Collaborator

mrchief commented Apr 2, 2019

This is great! Thanks for sending this!

@ellinge ellinge mentioned this pull request Apr 4, 2019
1 task
@mrchief
Copy link
Collaborator

mrchief commented Apr 5, 2019

Screen Shot 2019-04-05 at 4 45 09 PM

aria-hidden seems incorrect here as this is not hidden content. Also, I think we should add aria-expanded here as it marks collapsible content.

@mrchief
Copy link
Collaborator

mrchief commented Apr 5, 2019

Screen Shot 2019-04-05 at 4 50 56 PM

The root should be level 0, instead it's level 1

@mrchief
Copy link
Collaborator

mrchief commented Apr 24, 2019

Don't bother resolving the intermediate conflicts. We can do a final resolving when this PR is about to be merged. Same goes for 230, 242

ellinge and others added 8 commits April 27, 2019 22:24
@mrchief
Copy link
Collaborator

mrchief commented May 1, 2019

Need to figure out a way to resolve the codeclimate complexity error. I think CC is right here. Gonna take a shot with fresh mind tomorrow. Feel free to propose any ideas you may have.

@ellinge
Copy link
Collaborator

ellinge commented May 1, 2019

Separated out the trigger to a separate js. Two large methods which could be moved as well.

@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 791ff75 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Collaborator

@mrchief mrchief left a comment

Choose a reason for hiding this comment

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

👍

@mrchief mrchief merged commit 7133ed2 into dowjones:develop May 3, 2019
mrchief pushed a commit that referenced this pull request May 3, 2019
@mrchief
Copy link
Collaborator

mrchief commented May 4, 2019

@allcontributors[bot] Let's add @gazab for code

@allcontributors
Copy link
Contributor

@mrchief

I've put up a pull request to add @gazab! 🎉

@mrchief
Copy link
Collaborator

mrchief commented Jun 10, 2019

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants