-
Notifications
You must be signed in to change notification settings - Fork 313
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
Change sync button title attribute #2814
Conversation
@burhandodhy instead of simply "Sync", let's say "Sync Page" (and "Settings Page".) |
1 similar comment
@burhandodhy instead of simply "Sync", let's say "Sync Page" (and "Settings Page".) |
describe('Settings Page', () => { | ||
it('Can see a Sync and Settings buttons on Settings Page', () => { | ||
cy.visitAdminPage('admin.php?page=elasticpress-settings'); | ||
cy.get('.dashicons.start-sync').should('have.attr', 'title', 'Sync Page'); | ||
cy.get('.dashicons.dashicons-admin-generic').should('have.attr', 'title', 'Settings Page'); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is GREAT to see tests like these coming through, @burhandodhy, congrats, and thanks! Can we please move it from a whole new test to the already existent general.spec.js
? The links should be on all (or almost all) pages, so although it makes sense to pick one to check, we can save a settings-page.spec.js
to something particular to that page, like checking if tabs change, etc. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felipeelia agree with you. settings-page.spec.js
should only have tests that are related to settings page. I move the test to general.spec.js
Description of the Change
This PR changes the Sync button title attribute to
Sync Page
and Settings button title toSettings Page
and also add the E2E test.Closes #2793
Verification Process
Sync
Changelog Entry
Changed: Sync button Title.
Credits
Props @burhandodhy @JakePT