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

refactor(locators): Locators have been replaced with more resilient ones to better handle layout changes. #107

Merged
merged 11 commits into from
Oct 25, 2024

Conversation

chalapkoStanislav
Copy link
Collaborator

Locators have been replaced with more resilient ones to better handle layout changes.

Updated screenshots according to the new locators
Modified tests according to the new locators
Added fixes to improve test stability
Updated the README as some information was outdated.

Copy link
Collaborator

@tdelatorre tdelatorre left a comment

Choose a reason for hiding this comment

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

Good job! I've put some comments. Two more suggestions:

  • I've read the commit messages and in this case it's better to have a single commit for the whole PR.

  • I've run the tests and some of them failed. Could you please check it?

Thanks!

pages/workspace/design-panel-page.js Outdated Show resolved Hide resolved
pages/workspace/design-panel-page.js Outdated Show resolved Hide resolved
pages/workspace/view-mode-page.js Outdated Show resolved Hide resolved
@tdelatorre tdelatorre removed the request for review from daniel-herrero October 23, 2024 11:00
@chalapkoStanislav
Copy link
Collaborator Author

Ran the tests, 7 failed because:

6 tests failed due to new accounts landing directly in the workspace instead of the dashboard (this changed today on PRE).
1448 - screenshot.

@tdelatorre

This comment was marked as resolved.

@tdelatorre
Copy link
Collaborator

Changes reviewed! Just one task and it's approved: a single commit for the whole PR.

Copy link
Collaborator

@tdelatorre tdelatorre left a comment

Choose a reason for hiding this comment

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

Condition: you should have a single commit for the whole PR (because all commits are about the same thing)

@chalapkoStanislav chalapkoStanislav merged commit f7352f7 into main Oct 25, 2024
1 check failed
@chalapkoStanislav chalapkoStanislav deleted the refactor-locators branch October 29, 2024 10:05
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