Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Update spaces.spec.ts - use Cypress Testing Library #10620

Merged
merged 1 commit into from
Apr 20, 2023
Merged

Update spaces.spec.ts - use Cypress Testing Library #10620

merged 1 commit into from
Apr 20, 2023

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Apr 15, 2023

For element-hq/element-web#25033

This PR update spaces.spec.ts by using Cypress Testing Library.

type: task

Signed-off-by: Suguru Hirahara luixxiul@users.noreply.github.com

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

@github-actions github-actions bot added Z-Community-PR Issue is solved by a community member's PR T-Task Refactoring, enabling or disabling functionality, other engineering tasks labels Apr 15, 2023
<AccessibleButton
className={classNames("mx_SpaceCreateMenuType", className)}
onClick={onClick}
aria-label={_t("Create a %(publicOrPrivate)s space", {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the aria label different from the button text contents? This button already has a good accessible name. Our testing utils should be flexible enough to select buttons in multiple ways as needed.

This comment was marked as outdated.

@@ -62,6 +63,7 @@ const SpacePublicShare: React.FC<IProps> = ({ space, onFinished }) => {
if (onFinished) onFinished();
showRoomInviteDialog(space.roomId);
}}
aria-label={_t("Invite people")}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't you use the text in the button itself to select this for tests?

This comment was marked as outdated.

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
@luixxiul luixxiul marked this pull request as ready for review April 20, 2023 12:13
@luixxiul luixxiul requested a review from a team as a code owner April 20, 2023 12:13
@luixxiul luixxiul changed the title Update spaces.spec.ts - use Cypress Testing Library and set unique aria-label Update spaces.spec.ts - use Cypress Testing Library Apr 20, 2023
cy.get(".mx_SpacePanel:not(.collapsed)").should("exist");
cy.findByRole("tree", { name: "Spaces" }).within(() => {
// This finds the expand button with the class name "mx_SpaceButton_toggleCollapse". Note there is another
// button with the same name with different class name "mx_SpacePanel_toggleCollapse".
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is a bit confusing, is the second class meant to be different?

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 is intended to note that there are two buttons whose name is Expand and it is not possible to select the proper one without specifying with findByRole("tree", { name: "Spaces" }).within.

@kerryarchibald kerryarchibald added this pull request to the merge queue Apr 20, 2023
Merged via the queue into matrix-org:develop with commit 5445ee8 Apr 20, 2023
@luixxiul luixxiul deleted the test-spaces branch April 21, 2023 04:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants