From 251aecc58f83478cd3db4d249f5ec799f989677d Mon Sep 17 00:00:00 2001 From: Muhammed Mustafa Date: Sat, 18 Feb 2023 22:16:35 +0200 Subject: [PATCH] feat(client): change the save buttons from disabled to aria-disabled (#49216) * feat(client): change the buttons from disabled to aria-disabled Co-authored-by: Bruce Blaser * stop the API call when the validation isn't true * use aria disabled instead of disabled on the test? * use have.attr to check for aria disabled * hide the button when it's disabled Co-authored-by: Bruce B * update Privacy Settings save button --------- Co-authored-by: Bruce Blaser --- .../src/components/helpers/toggle-button.css | 20 ++++++++++ client/src/components/settings/about.tsx | 7 +++- client/src/components/settings/email.tsx | 19 ++++++---- client/src/components/settings/honesty.css | 11 ------ client/src/components/settings/internet.tsx | 6 +-- client/src/components/settings/portfolio.tsx | 38 ++++++++++--------- client/src/components/settings/privacy.tsx | 3 +- client/src/components/settings/username.tsx | 9 ++--- .../e2e/default/settings/username-change.ts | 22 ++++++----- 9 files changed, 80 insertions(+), 55 deletions(-) diff --git a/client/src/components/helpers/toggle-button.css b/client/src/components/helpers/toggle-button.css index 24d347baf17..50cf69f9f31 100644 --- a/client/src/components/helpers/toggle-button.css +++ b/client/src/components/helpers/toggle-button.css @@ -4,6 +4,26 @@ opacity: 1; } +:is( + .about-settings, + .privacy-settings, + .email-settings, + #usernameSettings, + #camper-identity, + #internet-presence, + #portfolio-items, + #honesty-policy + ) + :is( + button[aria-disabled='true'], + button[aria-disabled='true']:is(:focus, :hover) + ) { + background-color: var(--quaternary-background); + color: var(--secondary-color); + opacity: 0.65; + cursor: not-allowed; +} + .toggle-not-active { background-color: var(--quaternary-background); color: var(--secondary-color); diff --git a/client/src/components/settings/about.tsx b/client/src/components/settings/about.tsx index ab8f1b96dfa..7df7827dc35 100644 --- a/client/src/components/settings/about.tsx +++ b/client/src/components/settings/about.tsx @@ -118,7 +118,7 @@ class AboutSettings extends Component { e.preventDefault(); const { formValues } = this.state; const { submitNewAbout } = this.props; - if (this.state.isPictureUrlValid === true) { + if (this.state.isPictureUrlValid === true && !this.isFormPristine()) { return this.setState({ formClicked: true }, () => submitNewAbout(formValues) ); @@ -259,7 +259,10 @@ class AboutSettings extends Component { /> - + {t('buttons.save')}{' '} {t('settings.headings.personal-info')} diff --git a/client/src/components/settings/email.tsx b/client/src/components/settings/email.tsx index 79b8f3d6725..6be591f53cb 100644 --- a/client/src/components/settings/email.tsx +++ b/client/src/components/settings/email.tsx @@ -132,7 +132,10 @@ function EmailSettings({ state: confirmEmailValidation, message: confirmEmailValidationMessage } = getValidationForConfirmEmail(); - + const isDisabled = + newEmailValidation !== 'success' || + confirmEmailValidation !== 'success' || + isPristine; if (!currentEmail) { return (
@@ -170,7 +173,12 @@ function EmailSettings({ )} -
+ e.preventDefault() })} + > {t('settings.email.current')} {currentEmail} @@ -206,11 +214,8 @@ function EmailSettings({
{t('buttons.save')}{' '} {t('settings.email.heading')} diff --git a/client/src/components/settings/honesty.css b/client/src/components/settings/honesty.css index 979df5c6c86..dd0d550b49f 100644 --- a/client/src/components/settings/honesty.css +++ b/client/src/components/settings/honesty.css @@ -1,14 +1,3 @@ -#honesty-policy - :is( - button[aria-disabled='true'], - button[aria-disabled='true']:is(:focus, :hover) - ) { - background-color: var(--quaternary-background); - color: var(--secondary-color); - opacity: 0.65; - cursor: not-allowed; -} - .honesty-panel p { margin-inline: 10px; font-family: 'Lato', sans-serif; diff --git a/client/src/components/settings/internet.tsx b/client/src/components/settings/internet.tsx index 15ae0c1928e..a9c4c3b9faf 100644 --- a/client/src/components/settings/internet.tsx +++ b/client/src/components/settings/internet.tsx @@ -66,7 +66,6 @@ class InternetSettings extends Component { twitter !== originalValues.twitter || website !== originalValues.website ) { - // eslint-disable-next-line react/no-did-update-set-state return this.setState({ originalValues: { githubProfile, linkedin, twitter, website } }); @@ -179,7 +178,7 @@ class InternetSettings extends Component { const { state: websiteValidation, message: websiteValidationMessage } = this.getValidationStateFor(website); - + const isDisabled = this.isFormPristine() || !this.isFormValid(); return ( <> {t('settings.headings.internet')} @@ -244,7 +243,8 @@ class InternetSettings extends Component { {t('buttons.save')}{' '} {t('settings.headings.internet')} diff --git a/client/src/components/settings/portfolio.tsx b/client/src/components/settings/portfolio.tsx index 00c20aa85f1..aae38a7acc6 100644 --- a/client/src/components/settings/portfolio.tsx +++ b/client/src/components/settings/portfolio.tsx @@ -84,11 +84,6 @@ class PortfolioSettings extends Component { }); }; - handleSubmit = (e: React.FormEvent, id: string) => { - e.preventDefault(); - this.updateItem(id); - }; - updateItem = (id: string) => { const { portfolio, unsavedItemId } = this.state; if (unsavedItemId === id) { @@ -218,9 +213,27 @@ class PortfolioSettings extends Component { ); const { state: descriptionState, message: descriptionMessage } = this.getDescriptionValidation(description); + + const isDisabled = + pristine || + !title || + !isURL(url, { + protocols: ['http', 'https'], + /* eslint-disable camelcase, @typescript-eslint/naming-convention */ + require_tld: true, + require_protocol: true + /* eslint-enable camelcase, @typescript-eslint/naming-convention */ + }); + + const handleSubmit = (e: React.FormEvent, id: string) => { + e.preventDefault(); + if (isDisabled) return null; + return this.updateItem(id); + }; + return ( - this.handleSubmit(e, id)}> + handleSubmit(e, id)} id='portfolio-items'> { ) : null} {t('buttons.save-portfolio')} diff --git a/client/src/components/settings/privacy.tsx b/client/src/components/settings/privacy.tsx index 81cea24d34f..992495a02ee 100644 --- a/client/src/components/settings/privacy.tsx +++ b/client/src/components/settings/privacy.tsx @@ -152,7 +152,8 @@ function PrivacySettings({ bsStyle='primary' data-cy='save-privacy-settings' block={true} - disabled={!madeChanges} + aria-disabled={!madeChanges} + {...(!madeChanges && { tabindex: -1 })} > {t('buttons.save')}{' '} {t('settings.headings.privacy')} diff --git a/client/src/components/settings/username.tsx b/client/src/components/settings/username.tsx index d7a6fd26e76..3356a129a6d 100644 --- a/client/src/components/settings/username.tsx +++ b/client/src/components/settings/username.tsx @@ -96,7 +96,6 @@ class UsernameSettings extends Component { const { username } = this.props; const { formValue } = this.state; if (prevUsername !== username && prevFormValue === formValue) { - // eslint-disable-next-line react/no-did-update-set-state return this.setState({ isFormPristine: username === formValue, submitClicked: false, @@ -210,7 +209,8 @@ class UsernameSettings extends Component { submitClicked } = this.state; const { isValidUsername, t, validating } = this.props; - + const isDisabled = + !(isValidUsername && valid && !isFormPristine) || submitClicked; return ( { this.renderAlerts(validating, error, isValidUsername)} {t('buttons.save')}{' '} {t('settings.labels.username')} diff --git a/cypress/e2e/default/settings/username-change.ts b/cypress/e2e/default/settings/username-change.ts index 55c048a648d..dc8a8a78e6c 100644 --- a/cypress/e2e/default/settings/username-change.ts +++ b/cypress/e2e/default/settings/username-change.ts @@ -43,12 +43,11 @@ describe('Username input field', () => { .should('have.class', 'alert alert-info'); }); - // eslint-disable-next-line it('Should be able to click the `Save` button if username is available', () => { cy.typeUsername('oliver'); cy.get('@usernameForm').within(() => { - cy.contains('Save').should('not.be.disabled'); + cy.contains('Save').should('have.attr', 'aria-disabled', 'false'); }); }); @@ -63,7 +62,6 @@ describe('Username input field', () => { .should('have.class', 'alert alert-warning'); }); - // eslint-disable-next-line it('Should not be possible to click the `Save` button if username is unavailable', () => { cy.typeUsername('twaha'); @@ -74,20 +72,25 @@ describe('Username input field', () => { 'the URL to your profile and your certifications.' ).should('not.exist'); - cy.get('@usernameForm').contains('Save').should('be.disabled'); + cy.get('@usernameForm') + .contains('Save') + .should('have.attr', 'aria-disabled', 'true'); }); it('Should not show anything if user types their current name', () => { cy.typeUsername('developmentuser'); - cy.get('@usernameForm').contains('Save').should('be.disabled'); + cy.get('@usernameForm') + .contains('Save') + .should('have.attr', 'aria-disabled', 'true'); }); - // eslint-disable-next-line max-len it('Should not be possible to click the `Save` button if user types their current name', () => { cy.typeUsername('developmentuser'); - cy.get('@usernameForm').contains('Save').should('be.disabled'); + cy.get('@usernameForm') + .contains('Save') + .should('have.attr', 'aria-disabled', 'true'); }); it('Should show warning if username includes invalid character', () => { @@ -101,11 +104,12 @@ describe('Username input field', () => { .should('have.class', 'alert alert-danger'); }); - // eslint-disable-next-line max-len it('Should not be able to click the `Save` button if username includes invalid character', () => { cy.typeUsername('Quincy Larson'); - cy.get('@usernameForm').contains('Save').should('be.disabled'); + cy.get('@usernameForm') + .contains('Save') + .should('have.attr', 'aria-disabled', 'true'); }); it('Should change username if `Save` button is clicked', () => {