From a0fbccf61280a7391caf3e28a6ad31451749086b Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 3 Sep 2021 09:42:56 +1000 Subject: [PATCH] Revert "A11Y: Improve create account modal for screen readers (#14204)" (#14233) This reverts commit e0d2de73d89cdea13e9681b2daaa52074ee510a5. --- app/assets/javascripts/discourse-loader.js | 3 - .../discourse/app/components/d-modal-body.js | 25 ++---- .../discourse/app/components/d-modal.js | 18 ++-- .../app/controllers/create-account.js | 30 +++---- .../discourse/app/controllers/login.js | 5 +- .../ember-input-component-extension.js | 11 --- .../discourse/app/lib/show-modal.js | 4 - .../discourse/app/mixins/name-validation.js | 8 +- .../app/mixins/password-validation.js | 10 +-- .../app/mixins/username-validation.js | 8 +- .../discourse/app/models/login-method.js | 5 -- .../discourse/app/routes/application.js | 13 +-- .../app/templates/components/d-modal.hbs | 2 +- .../templates/components/login-buttons.hbs | 2 +- .../discourse/app/templates/modal.hbs | 1 - .../app/templates/modal/create-account.hbs | 82 ++++++------------- .../create-account-user-fields-test.js | 8 +- app/assets/stylesheets/common/base/login.scss | 5 +- config/locales/client.en.yml | 6 -- 19 files changed, 69 insertions(+), 177 deletions(-) delete mode 100644 app/assets/javascripts/discourse/app/initializers/ember-input-component-extension.js diff --git a/app/assets/javascripts/discourse-loader.js b/app/assets/javascripts/discourse-loader.js index 54cef936e60..f903dbe4918 100644 --- a/app/assets/javascripts/discourse-loader.js +++ b/app/assets/javascripts/discourse-loader.js @@ -163,9 +163,6 @@ var define, requirejs; "@ember/object/internals": { guidFor: Ember.guidFor, }, - "@ember/views/text-support": { - default: Ember.TextSupport, - }, I18n: { // eslint-disable-next-line default: I18n, diff --git a/app/assets/javascripts/discourse/app/components/d-modal-body.js b/app/assets/javascripts/discourse/app/components/d-modal-body.js index 1e46c4442a8..2ea7d11324d 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal-body.js +++ b/app/assets/javascripts/discourse/app/components/d-modal-body.js @@ -8,10 +8,7 @@ export default Component.extend({ didInsertElement() { this._super(...arguments); - this._modalAlertElement = document.getElementById("modal-alert"); - if (this._modalAlertElement) { - this._modalAlertElement.innerHTML = ""; - } + $("#modal-alert").hide(); let fixedParent = $(this.element).closest(".d-modal.fixed-modal"); if (fixedParent.length) { @@ -58,10 +55,10 @@ export default Component.extend({ }, _clearFlash() { - if (this._modalAlertElement) { - this._modalAlertElement.innerHTML = ""; - this._modalAlertElement.classList.remove( - "alert", + const modalAlert = document.getElementById("modal-alert"); + if (modalAlert) { + modalAlert.style.display = "none"; + modalAlert.classList.remove( "alert-error", "alert-info", "alert-success", @@ -72,14 +69,10 @@ export default Component.extend({ _flash(msg) { this._clearFlash(); - if (!this._modalAlertElement) { - return; - } - this._modalAlertElement.classList.add( - "alert", - `alert-${msg.messageClass || "success"}` - ); - this._modalAlertElement.innerHTML = msg.text || ""; + $("#modal-alert") + .addClass(`alert alert-${msg.messageClass || "success"}`) + .html(msg.text || "") + .fadeIn(); }, }); diff --git a/app/assets/javascripts/discourse/app/components/d-modal.js b/app/assets/javascripts/discourse/app/components/d-modal.js index 941a0ca556e..a08ec543e95 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal.js +++ b/app/assets/javascripts/discourse/app/components/d-modal.js @@ -1,7 +1,8 @@ +import { computed } from "@ember/object"; import Component from "@ember/component"; import I18n from "I18n"; import { next, schedule } from "@ember/runloop"; -import discourseComputed, { bind, on } from "discourse-common/utils/decorators"; +import { bind, on } from "discourse-common/utils/decorators"; export default Component.extend({ classNameBindings: [ @@ -20,7 +21,6 @@ export default Component.extend({ submitOnEnter: true, dismissable: true, title: null, - titleAriaElementId: null, subtitle: null, role: "dialog", headerClass: null, @@ -41,17 +41,9 @@ export default Component.extend({ // Inform screenreaders of the modal "aria-modal": "true", - @discourseComputed("title", "titleAriaElementId") - ariaLabelledby(title, titleAriaElementId) { - if (titleAriaElementId) { - return titleAriaElementId; - } - if (title) { - return "discourse-modal-title"; - } - - return; - }, + ariaLabelledby: computed("title", function () { + return this.title ? "discourse-modal-title" : null; + }), @on("didInsertElement") setUp() { diff --git a/app/assets/javascripts/discourse/app/controllers/create-account.js b/app/assets/javascripts/discourse/app/controllers/create-account.js index b58957450dc..52a3f7230a7 100644 --- a/app/assets/javascripts/discourse/app/controllers/create-account.js +++ b/app/assets/javascripts/discourse/app/controllers/create-account.js @@ -140,19 +140,16 @@ export default Controller.extend( "serverAccountEmail", "serverEmailValidation", "accountEmail", - "rejectedEmails.[]", - "forceValidationReason" + "rejectedEmails.[]" ) emailValidation( serverAccountEmail, serverEmailValidation, email, - rejectedEmails, - forceValidationReason + rejectedEmails ) { const failedAttrs = { failed: true, - ok: false, element: document.querySelector("#new-account-email"), }; @@ -165,9 +162,6 @@ export default Controller.extend( return EmberObject.create( Object.assign(failedAttrs, { message: I18n.t("user.email.required"), - reason: forceValidationReason - ? I18n.t("user.email.required") - : null, }) ); } @@ -432,7 +426,6 @@ export default Controller.extend( createAccount() { this.clearFlash(); - this.set("forceValidationReason", true); const validation = [ this.emailValidation, this.usernameValidation, @@ -442,22 +435,23 @@ export default Controller.extend( ].find((v) => v.failed); if (validation) { + if (validation.message) { + this.flash(validation.message, "error"); + } + const element = validation.element; - if (element) { - if (element.tagName === "DIV") { - if (element.scrollIntoView) { - element.scrollIntoView(); - } - element.click(); - } else { - element.focus(); + if (element.tagName === "DIV") { + if (element.scrollIntoView) { + element.scrollIntoView(); } + element.click(); + } else { + element.focus(); } return; } - this.set("forceValidationReason", false); this.performAccountCreation(); }, }, diff --git a/app/assets/javascripts/discourse/app/controllers/login.js b/app/assets/javascripts/discourse/app/controllers/login.js index a3a8934e522..fe6b32b757a 100644 --- a/app/assets/javascripts/discourse/app/controllers/login.js +++ b/app/assets/javascripts/discourse/app/controllers/login.js @@ -428,10 +428,7 @@ export default Controller.extend(ModalFunctionality, { }); next(() => { - showModal("createAccount", { - modalClass: "create-account", - titleAriaElementId: "create-account-title", - }); + showModal("createAccount", { modalClass: "create-account" }); }); }, }); diff --git a/app/assets/javascripts/discourse/app/initializers/ember-input-component-extension.js b/app/assets/javascripts/discourse/app/initializers/ember-input-component-extension.js deleted file mode 100644 index c909a4e86dd..00000000000 --- a/app/assets/javascripts/discourse/app/initializers/ember-input-component-extension.js +++ /dev/null @@ -1,11 +0,0 @@ -import TextSupport from "@ember/views/text-support"; - -export default { - name: "ember-input-component-extensions", - - initialize() { - TextSupport.reopen({ - attributeBindings: ["aria-describedby", "aria-invalid"], - }); - }, -}; diff --git a/app/assets/javascripts/discourse/app/lib/show-modal.js b/app/assets/javascripts/discourse/app/lib/show-modal.js index dde3094b315..db3067fbcf5 100644 --- a/app/assets/javascripts/discourse/app/lib/show-modal.js +++ b/app/assets/javascripts/discourse/app/lib/show-modal.js @@ -47,10 +47,6 @@ export default function (name, opts) { modalController.set("title", null); } - if (opts.titleAriaElementId) { - modalController.set("titleAriaElementId", opts.titleAriaElementId); - } - if (opts.panels) { modalController.setProperties({ panels: opts.panels, diff --git a/app/assets/javascripts/discourse/app/mixins/name-validation.js b/app/assets/javascripts/discourse/app/mixins/name-validation.js index 65287c97325..52623f537e3 100644 --- a/app/assets/javascripts/discourse/app/mixins/name-validation.js +++ b/app/assets/javascripts/discourse/app/mixins/name-validation.js @@ -15,14 +15,12 @@ export default Mixin.create({ }, // Validate the name. - @discourseComputed("accountName", "forceValidationReason") - nameValidation(accountName, forceValidationReason) { - if (this.siteSettings.full_name_required && isEmpty(accountName)) { + @discourseComputed("accountName") + nameValidation() { + if (this.siteSettings.full_name_required && isEmpty(this.accountName)) { return EmberObject.create({ failed: true, - ok: false, message: I18n.t("user.name.required"), - reason: forceValidationReason ? I18n.t("user.name.required") : null, element: document.querySelector("#new-account-name"), }); } diff --git a/app/assets/javascripts/discourse/app/mixins/password-validation.js b/app/assets/javascripts/discourse/app/mixins/password-validation.js index eea9b942de3..feb020cbf7a 100644 --- a/app/assets/javascripts/discourse/app/mixins/password-validation.js +++ b/app/assets/javascripts/discourse/app/mixins/password-validation.js @@ -33,8 +33,7 @@ export default Mixin.create({ "rejectedPasswords.[]", "accountUsername", "accountEmail", - "passwordMinLength", - "forceValidationReason" + "passwordMinLength" ) passwordValidation( password, @@ -42,12 +41,10 @@ export default Mixin.create({ rejectedPasswords, accountUsername, accountEmail, - passwordMinLength, - forceValidationReason + passwordMinLength ) { const failedAttrs = { failed: true, - ok: false, element: document.querySelector("#new-account-password"), }; @@ -70,9 +67,6 @@ export default Mixin.create({ return EmberObject.create( Object.assign(failedAttrs, { message: I18n.t("user.password.required"), - reason: forceValidationReason - ? I18n.t("user.password.required") - : null, }) ); } diff --git a/app/assets/javascripts/discourse/app/mixins/username-validation.js b/app/assets/javascripts/discourse/app/mixins/username-validation.js index 522b40ad5f0..b8f14bc818d 100644 --- a/app/assets/javascripts/discourse/app/mixins/username-validation.js +++ b/app/assets/javascripts/discourse/app/mixins/username-validation.js @@ -11,7 +11,6 @@ function failedResult(attrs) { let result = EmberObject.create({ shouldCheck: false, failed: true, - ok: false, element: document.querySelector("#new-account-username"), }); result.setProperties(attrs); @@ -61,12 +60,7 @@ export default Mixin.create({ } if (isEmpty(username)) { - return failedResult({ - message: I18n.t("user.username.required"), - reason: this.forceValidationReason - ? I18n.t("user.username.required") - : null, - }); + return failedResult({ message: I18n.t("user.username.required") }); } if (username.length < this.siteSettings.min_username_length) { diff --git a/app/assets/javascripts/discourse/app/models/login-method.js b/app/assets/javascripts/discourse/app/models/login-method.js index bebb069b93d..efe7c438bc8 100644 --- a/app/assets/javascripts/discourse/app/models/login-method.js +++ b/app/assets/javascripts/discourse/app/models/login-method.js @@ -13,11 +13,6 @@ const LoginMethod = EmberObject.extend({ return this.title_override || I18n.t(`login.${this.name}.title`); }, - @discourseComputed - screenReaderTitle() { - return this.title_override || I18n.t(`login.${this.name}.sr_title`); - }, - @discourseComputed prettyName() { return this.pretty_name_override || I18n.t(`login.${this.name}.name`); diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js index a283404cf58..4f9558c292f 100644 --- a/app/assets/javascripts/discourse/app/routes/application.js +++ b/app/assets/javascripts/discourse/app/routes/application.js @@ -267,18 +267,11 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, { const returnPath = encodeURIComponent(window.location.pathname); window.location = getURL("/session/sso?return_path=" + returnPath); } else { - this._autoLogin("createAccount", "create-account", { - signup: true, - titleAriaElementId: "create-account-title", - }); + this._autoLogin("createAccount", "create-account", { signup: true }); } }, - _autoLogin( - modal, - modalClass, - { notAuto = null, signup = false, titleAriaElementId = null } = {} - ) { + _autoLogin(modal, modalClass, { notAuto = null, signup = false } = {}) { const methods = findAll(); if (!this.siteSettings.enable_local_logins && methods.length === 1) { @@ -286,7 +279,7 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, { signup: signup, }); } else { - showModal(modal, { titleAriaElementId }); + showModal(modal); this.controllerFor("modal").set("modalClass", modalClass); if (notAuto) { notAuto(); diff --git a/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs b/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs index a0da06d06a7..7067f487ef8 100644 --- a/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs @@ -30,7 +30,7 @@ {{/if}} - + {{yield}} diff --git a/app/assets/javascripts/discourse/app/templates/components/login-buttons.hbs b/app/assets/javascripts/discourse/app/templates/components/login-buttons.hbs index c4e553f5b1d..bc2bc0aba98 100644 --- a/app/assets/javascripts/discourse/app/templates/components/login-buttons.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/login-buttons.hbs @@ -1,5 +1,5 @@ {{#each buttons as |b|}} -