Revamp the API and configuration to allow for more modular rules to be added#119
Conversation
romeovs
changed the title
Added <svg> and <path>
Adding some tags!
…bundle gets included in the npm package. Also disabled the release script and removed the rf-release package as this does not seem to be tagging or publishing to npm properly.
…ess and added descriptions of the scripts in the CONTRIBUTING.md
Publish improvements
|
Wow! Huge diff for review. Is the PR title still accurate? |
2. Added the AirBnB eslint config to inherit 3. Fixed all linter findings
Updated eslint and fixed findings
|
@romeovs looks like we have some merge conflicts to resolve but this looks amazing. You think it's sem-ver minor? |
|
The title is not accurate anymore. It could be more along the lines of:
The PR probably started out as moderate as fixing the includeSourceNode issue, but I was having fun expanding the codebase. It's semver major for, sure since the configuration changed quite a bit. Note that I'm currently not actively maintaining this branch anymore, but @erin-doyle and his team kindly volunteered pick up (only just last week) and they're doing a great job at getting this thing more production ready. I can explain the motivations behind some of the changes I've made if you want. |
|
can you guys merge this ? |
|
If I can be provided with write access I will be happy to resolve the conflicts and merge. |
… actually undefined.
Fixed Issue #16
|
@erin-doyle do you have the access you need? |
…Now it should pass if onChange is used in addition to onBlur.
…omments since the last tag to the CHANGELOG.
… actually undefined.
It appears as is ReactDOM has been removed as of react v16. As a result, react-a11y throws the error "react-a11y: missing argument 'ReactDOM'". Updating the option check to look for `findDOMNode` instead.
Missed one in the previous PR.
# Conflicts: # CHANGELOG.md # README.md # lib/index.js
|
Ugh sorry that this added a ton of "new" commits, I did a rebase onto this repo's master in order to resolve the conflicts. Also, I added an update to the |
…ng on node 4 that babel-register module is not found
|
Ok fixed! Any chance we can get this merged and put out a v1.0.0? |
|
@wyattdanger I know you're busy and don't mean to be a nag but my team is doing a lot of a11y work right now and we have some excitement for working on this lib and enhancing it even more. So if you have a moment to consider merging this and providing me permissions to be a maintainer, I'd really love to be able to do more with this great repo. |
|
@erin-doyle sorry i had been traveling and away from computers. i'll get to this this week, wednesday! y'all still have the interest in this? |
|
OK I have it checked out locally and passing all tests! Can you go ahead and give me a tldr for a changelog entry, and then I'll cut the release? Edit: Please update the title of the PR to reflect the changes, I believe that's where the changelog entry comes from Edit 2: Just saw the CHANGELOG entries in the diff. This looks like a semver major release? |
|
Went ahead and sent y'all invites to maintain AND to publish, so feel free to take it from here! Thanks for contributing all of this work! |
|
Yes I got the invite and we are definitely still interested, thanks @wyattdanger ! This is definitely semver major. The API has changed completely. So I can totally update this PR and then put together a release if you'd like. |
erin-doyle
changed the title
|
Good to see that this was merged! Thanks @erin-doyle and her team for putting in so much effort into this PR! |
9 participants
This doesn't really fix anything, but at least it alleviates the problems in #77, $79 and #85, reducing their scope to the case where the
includeSrcNodeoption is set totrue.It could be used as a quick fix before #54 or #55 get implemented. It does not change the external API as far as I can tell.