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

fix(get-role): handle presentation role inheritance for vnodes with no parent #3801

Conversation

dbowling
Copy link
Contributor

Closes issue: #3788

@dbowling dbowling marked this pull request as ready for review November 28, 2022 23:06
@dbowling dbowling requested a review from a team as a code owner November 28, 2022 23:06
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.

LGTM, although it'd be good to add a couple more tests to prove this out.

@@ -1,17 +1,61 @@
describe('empty-table-header virtual-rule', function () {
it('should incomplete when children are missing', function () {
it('should fail when children contain no visible text', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a couple more tests:

With empty content:

  1. it passes when the table has role=none
  2. it passes when the th has role=cell

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it passes when the table has role=none

Steve said that the following test needs to be a new ticket as it is not a related issue. He was going to chat with you today about it I think.

This test currently fails:

it('should pass with a table of role none', function () {
	var table = new axe.SerialVirtualNode({
		nodeName: 'table',
		attributes: {
			role: 'none'
		}
	});

	var tr = new axe.SerialVirtualNode({
		nodeName: 'tr',
	});

	var th = new axe.SerialVirtualNode({
		nodeName: 'th',
	});

	tr.children = [th];
	tr.parent = table;
	th.parent = tr;
	th.children = [];
	table.children = [tr];

	var results = axe.runVirtualRule('empty-table-header', table);

	assert.lengthOf(results.passes, 1);
	assert.lengthOf(results.violations, 0);
	assert.lengthOf(results.incomplete, 0);
});

it passes when the th has role=cell

Test has been added and passes.

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.

Please open that follow-up issue on table[role=none].

@dbowling
Copy link
Contributor Author

dbowling commented Dec 6, 2022

Issue added, #3817

@dbowling dbowling merged commit b971caf into develop Dec 6, 2022
@dbowling dbowling deleted the 3788-empty-table-header-virtualised-rule-should-run-without-requiring-role branch December 6, 2022 18:46
This was referenced Dec 9, 2022
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