Skip to content
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

feat(empty-table-header): new rule to flag empty table headers #2811

Merged
merged 8 commits into from
Feb 26, 2021

Conversation

jlin95
Copy link
Contributor

@jlin95 jlin95 commented Feb 17, 2021

<< Describe the changes >>
New rule created for #2604

For initial review only!

Closes issue:

@jlin95 jlin95 requested a review from a team as a code owner February 17, 2021 23:01
Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together. It's a great start and just needs a few things.

lib/rules/aria-empty-table-header.json Outdated Show resolved Hide resolved
lib/rules/aria-empty-table-header.json Outdated Show resolved Hide resolved
Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. Just a few more changes.

lib/rules/aria-empty-table-header.json Outdated Show resolved Hide resolved
lib/rules/aria-empty-table-header.json Outdated Show resolved Hide resolved
@jlin95
Copy link
Contributor Author

jlin95 commented Feb 23, 2021

Out of curiosity, have been getting
image while trying to run the axe.js build locally.

I didn't make any changes to the Gruntfile/etc, just synced my fork with the develop branch...

@straker
Copy link
Contributor

straker commented Feb 23, 2021

Out of curiosity, have been getting
image while trying to run the axe.js build locally.

I didn't make any changes to the Gruntfile/etc, just synced my fork with the develop branch...

The develop script just constantly watches for changes to axe files and rebuilds the app or runs the tests. Looks like it's working but are you seeing something different?

@jlin95
Copy link
Contributor Author

jlin95 commented Feb 24, 2021

Out of curiosity, have been getting
image while trying to run the axe.js build locally.
I didn't make any changes to the Gruntfile/etc, just synced my fork with the develop branch...

The develop script just constantly watches for changes to axe files and rebuilds the app or runs the tests. Looks like it's working but are you seeing something different?

Yep, basically previously I would navigate to localhost:9876/test/integrations and run all the tests at one shot from the browser, but now it seems that each rule/etc will execute the tests individually instead of all at one shot, please correct me if I understood it wrongly!

@straker
Copy link
Contributor

straker commented Feb 24, 2021

Oh yes, I believe that was before we switched to karma for our testing framework. Develop now just watches and re-runs the specific test file associated with the script. To run the full tests you do npm test or there is an entire script setup to run individual directory tests (e.g. npm run test:unit:core to run core tests).

@straker straker changed the title Aria table header name [Review] feat(empty-table-header): new rule to flag empty table headers Feb 24, 2021
Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you for the contribution!

@WilcoFiers
Copy link
Contributor

Reviewed for security.

@WilcoFiers WilcoFiers merged commit 813ee7e into dequelabs:develop Feb 26, 2021
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.

None yet

3 participants