-
Notifications
You must be signed in to change notification settings - Fork 8
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
Move "Create new user" and "Change existing user" request forms to use GOV.UK Design System #1252
Closed
floehopper
wants to merge
23
commits into
main
from
move-create-user-and-change-user-forms-to-use-govuk-design-system
Closed
Move "Create new user" and "Change existing user" request forms to use GOV.UK Design System #1252
floehopper
wants to merge
23
commits into
main
from
move-create-user-and-change-user-forms-to-use-govuk-design-system
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
floehopper
force-pushed
the
move-create-user-and-change-user-forms-to-use-govuk-design-system
branch
from
January 23, 2024 13:32
add4075
to
af1ad28
Compare
We're planning to move the app to use the GOV.UK Design System and this is a small step in that direction.
We're planning to move the app to use the GOV.UK Design System and this is a small step in that direction.
We're planning to move the app to use the GOV.UK Design System and this is a small step in that direction.
And make its assets available via a new stylesheet: app/assets/stylesheets/application.scss
Trello: https://trello.com/c/aQiPdsiP I'm not using this layout anywhere yet - I want to build it up piece by piece to make things clearer. The plan is to gradually migrate each page to use this new layout.
I've modelled NavigationItemsHelper on a similar module in Signon [1]. [1]: https://github.com/alphagov/signon/blob/6dee5f489dae346d43a47d4c57a58de6e2ec401c/app/helpers/navigation_items_helper.rb
It's more idiomatic/readable to have one call to `validates` per attribute.
Now that we're displaying *per-attribute* validation errors for the create/change user request forms it would be confusing to see two validation errors for the `email` attribute when the attribute is blank: one triggered by the `presence` validation; and one triggered by the `format` validation. This adds the `allow_blank` option to the `format` validation, so that now that validation is only triggered if the `email` attribute is not blank.
Trello: https://trello.com/c/aQiPdsiP This makes the form more similar to the new design which will make it easier to move the form to use the GOV.UK Design System in a subsequent commit.
As requested by Etain.
Trello: https://trello.com/c/aQiPdsiP This overrides the default Rails validation error messages using the `config/locales/en.yml` file. The actual copy was provided by Etain. I've chosen to use the locale file rather than specify the custom messages in the `CreateNewUserRequest` model, because a number of them are quite long and it would've made the code less readable. I'm planning to use the `govuk_design_system_formbuilder` gem [1] when I move this form to use the GOV.UK Design System and for some reason that doesn't display the attribute name when displaying validation errors against an individual field, i.e. you see "can't be blank" against the "User's name" field rather than "Name can't be blank". Customizing the error messages like this seems to avoid that problem. Doing this also meant I was able to make the specs robust against changes to the copy. The latter and having the messages in one place should also make it easier for content designers to update the copy without help from developers. [1]: https://govuk-form-builder.netlify.app/
Trello: https://trello.com/c/aQiPdsiP I'm planning on using the govuk_design_system_formbuilder gem [1] to change forms to use the GOV.UK Design System. [1]: https://govuk-form-builder.netlify.app/
Trello: https://trello.com/c/aQiPdsiP * Use the recently added "design_system" layout and the `GOVUKDesignSystemFormBuilder::FormBuilder` in the `CreateNewUserRequestsController` so the design system changes are restricted to this one form. * Modify the `_request_details.html.erb` template and add an overriding `_collaborators.html.erb` template to use the new design and form builder for all the form elements. * Add an overriding `new.html.erb` to make use of the `#govuk_error_summary` and `#govuk_submit` form builder methods. I plan to make other changes in this template in subsequent commit which I think justify the small amount of duplication introduced at this stage. * Add a `content_for(:error_summary)` to the `design_system` layout so that it appears in the right place on the page. * Add a blank option to the controller `#organisation_options` method, because the `#govuk_collection_select` form builder method doesn't support an `allow_blank` option. * Modify and enhance tests for validation errors to highlight that errors are now displayed inline against the relevant form field as well as in an error summary at the top of the page. Note that we've lost the functionality provided by `select2` which previously allowed users with JavaScript enabled to search for organisations in the drop down. I plan to address that in a subsequent commit. Also note that there are no breadcrums in the new "design_system" layout. However, according [1] to the GOV.UK Design System, there is no need for them on this page, because it is a "top-level" page and its parent page, the home page, is available in the navigation links. [1]: https://design-system.service.gov.uk/components/breadcrumbs/
As requested by Etain.
As recommended [1] by the GOV.UK Design System. This is also a lot less useful now we have much more comprehensive error messages provided by the server-side validation. [1]: https://design-system.service.gov.uk/patterns/validation/#turn-off-html5-validation
From "Submit" -> "Send request" as requested by Etain.
Trello: https://trello.com/c/aQiPdsiP This uses the accessible-autocomplete component [1] to progressively enhance [2] the existing organisation dropdown so that users with JavaScript enabled can type into an input to search for the organisation name. The extra styles in `_autocomplete.scss` are intended to make the component fit into the GOV.UK Design System and were provided by our designer, Calum Ryan. The organisation dropdown is not a required field and the accessible-autocomplete component doesn't provide a way to "clear" the selection, so I've added a "None" option. Unfortunately if I make that the default option (instead of a blank option) then when the user first clicks on the component, they only see organisations matching "None" in the "search results" for the component. To address the latter I've still included the default blank option, which never shows up in the "search results" for the component. I then map the "None" option to the blank option in `CreateNewUserRequest#organisation`, so that the two options are effectively equivalent. Note that we did consider re-using the `autocomplete` component [3] from Whitehall, but that only seemed to allow *multiple* selections. We also considered re-using the `select_with_search` component [4] from Whitehall, but that relies on the choices.js library [5] and we weren't sure that it had been as rigorously tested for accessibility. [1]: https://github.com/alphagov/accessible-autocomplete [2]: https://github.com/alphagov/accessible-autocomplete#progressive-enhancement [3]: https://github.com/alphagov/whitehall/blob/0c72a9cbf768b4be8d312447b3b984007ed6fbad/app/views/components/_autocomplete.html.erb [4]: https://github.com/alphagov/whitehall/blob/0c72a9cbf768b4be8d312447b3b984007ed6fbad/app/views/components/_select_with_search.html.erb [5]: https://choices-js.github.io/Choices/
Strictly speaking this isn't needed at the moment. However, since some other controllers *do* set `flash[:alert]`, it makes sense to add it to the new design system layout now. This seems especially sensible since we're using `GOVUKDesignSystemFormBuilder::Builder#govuk_error_summary` to generate the error summary at the top of the page for the `CreateNewUserRequestsController#new` page and so we have to disable the default validation messages alert in `RequestsController#create`. If someone added the global alert to the design system layout later, they could easily miss the need to do this.
Strictly speaking this isn't needed at the moment. However, this re-implements some similar behaviour in the legacy layout and is in line with other applications which have been switched to use the GOV.UK Design System.
floehopper
force-pushed
the
move-create-user-and-change-user-forms-to-use-govuk-design-system
branch
from
January 25, 2024 16:00
af1ad28
to
71b54c1
Compare
Superseded by #1259. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trello: https://trello.com/c/aQiPdsiP