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

test(react-color-picker): Added cy and a11y tests for sliders #33609

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ValentinaKozlova
Copy link
Contributor

New Behavior

Added tests for AlphaSlider and ColorSlider

  • A11y tests
  • Keyboard navigation
  • Mouse interaction

@ValentinaKozlova ValentinaKozlova self-assigned this Jan 10, 2025
@ValentinaKozlova ValentinaKozlova marked this pull request as ready for review January 10, 2025 16:46
@ValentinaKozlova ValentinaKozlova requested a review from a team as a code owner January 10, 2025 16:46
@ValentinaKozlova ValentinaKozlova mentioned this pull request Jan 10, 2025
28 tasks
Copy link

github-actions bot commented Jan 10, 2025

📊 Bundle size report

✅ No changes found

Copy link

Pull request demo site: URL

});

describe('mouse navigation', () => {
it('has correct a11y attributes', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please clarify what this test verifies? Does the value change before or after a mouse click?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Value changes after mouse click. It should check that a11y attributes before click and after

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks a bit odd as you are verifying different attributes before/after mouse click

const [color, setColor] = React.useState(props.color ?? { h: 0, s: 1, v: 1 });
return (
<>
<p tabIndex={0} id="before">
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, why is #before part of the component and #after isn't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's needed for the navigation. And #after is used only once checking focus behaviors

});

describe('mouse navigation', () => {
it('has correct a11y attributes', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The same question as above, what does this test verifies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The correct a11y values before and after mouse click

cy.realPress('Tab');
cy.realPress('ArrowLeft');
cy.realPress('ArrowLeft');
cy.get('#alpha-slider').should('have.attr', 'aria-valuetext', '48%');
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if it's a common pattern across the repo, but we could create a function for this, and reuse it:

function assertSliderValue(value: string) {
  cy.get('#alpha-slider').should('have.attr', 'aria-valuetext', `${value}%`);
  cy.get('#alpha-slider').should('have.attr', 'value', value);
}


// and then

expectSliderValue('48')

cy.realPress('ArrowRight');
cy.get('#alpha-slider').should('have.attr', 'value', '29');
cy.realPress('ArrowUp');
cy.get('#alpha-slider').should('have.attr', 'value', '30');
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we use the assertSliderValue for all cases?

});

describe('mouse navigation', () => {
it('has correct a11y attributes', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks a bit odd as you are verifying different attributes before/after mouse click

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

Successfully merging this pull request may close these issues.

3 participants