Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

[3155] Fix error styling and standardise behaviour across the service #222

Merged
merged 7 commits into from
Mar 25, 2020
Merged

[3155] Fix error styling and standardise behaviour across the service #222

merged 7 commits into from
Mar 25, 2020

Conversation

aliuk2012
Copy link
Contributor

@aliuk2012 aliuk2012 commented Mar 23, 2020 •
edited
Loading

Background

During migration from c# to Rails, a lot of different developers worked on various pages and were mostly trying to replicate what was done in c#. Unfortunately, this introduced some inconsistency in styling, error messages and interaction when the user clicked on the error summary link.

Changes proposed in this pull request

Home/Location page

  • Add missing error message when no type of location search is selected [e41e60a]
  • reorder the hint/error message so that it matches govuk-frontend guidelines [40705f1]
  • fixes malformed HTML where two elements on the page had the same id. This was done by excluding the keys in the hidden fields. (No visual difference)[d204866]
  • fixes an accessibility issue with location/provider labels and text fields were not associated so screenreaders would not be able to provide the context of the input to the user. (No visual difference but users should be able to click on the labels)[e20d44e]
  • Error summary link should set focus on the first radio/checkbox in a list or the invalid text field [d986e40]

Study type filter page

  • Error summary link should set focus on the first radio/checkbox in a list or the invalid text field [d986e40]
  • Does not set the default to both full time/parttime if there is an error. This fixes a usability issue where a user may untick full-time and part-time checkboxes, they get an error message saying they need to select at least one but both checkboxes are ticked already.[d8083f0]

Qualification filter page

  • Error summary link should set focus on the first radio/checkbox in a list or the invalid text field (No visual difference)[04409e2]

Screenshots

Before - Home/Location page (3 error states)

image

image
image

After - Home/Location page (3 error states)

image

image

image

Before - Study type filter page

image

After - Study type filter page

image

Outstanding issues not addressed in this PR

Home/Location page

Subject page

  • Error summary does not link to the first checkbox because accordion sections do not expand when expected. This is because accordion has a feature that stores the state of each section in session storage. The session storage overrides the state of the accordion that the server has sent.
  • Checkboxes do not have error style i.e red left border and indent (I did try to apply it across the whole accordion but it didn't feel like it added much value to the accordion.

Generally, the subject page is complicated because we are using an accordion to group what would have been a large list of checkboxes into different groups.

Checklist

  • Make sure all information from the Trello card is in here
  • Attach to Trello card
  • Rebased master
  • Cleaned commit history
  • Tested by running locally
  • Product review

@auto-comment
Copy link

auto-comment bot commented Mar 23, 2020

Auto pull request trigger is configured on this repository for PR validation. To review the app deployed by this PR, please replace #### in the URL given below with the first four characters of your branch name. This could be your trello card number. https://s121d02-####-find-as.azurewebsites.net/

@dankmitchell dankmitchell temporarily deployed to find-teacher-3155-fix-e-3pbmmm March 23, 2020 10:44 Inactive
@aliuk2012 aliuk2012 force-pushed the 3155-fix-error-message-inconsistency-design-review branch from f77e421 to 1048534 Compare March 24, 2020 14:11
@aliuk2012 aliuk2012 temporarily deployed to find-teacher-3155-fix-e-3pbmmm March 24, 2020 14:11 Inactive
@aliuk2012 aliuk2012 changed the title [3155] Fix error styling on home page [3155] Fix error styling and standardise behaviour across the service Mar 24, 2020
@aliuk2012 aliuk2012 force-pushed the 3155-fix-error-message-inconsistency-design-review branch from 1048534 to 86d493a Compare March 25, 2020 07:49
@aliuk2012 aliuk2012 temporarily deployed to find-teacher-3155-fix-e-3pbmmm March 25, 2020 07:50 Inactive
@aliuk2012
[3155] Add missing error message/styling
e41e60a 
Home/location page did not follow the styling convention we use throughout
the service and recommended by GOV.UK Design System.
@aliuk2012
[3155] reorder provider lookup hint/error message
40705f1 
Reorders the hint/error message so that it matches govuk-frontend
guidelines of label, hint, error.
@aliuk2012
[3155] Remove elements with duplicate id
d204866 
I discovered there where elements on the page had the same id.
THis causes a couple of issues

- Elements on a page should have unique ID
- Field labels did not work because label did not know which element
  to focus on (Accessibility issue)

What was causing this is that we output a bunch of hidden fields that
represent params which we track throughout the users journey. We should
have excluded any form fields that were being used on the page.
@aliuk2012
[3155] Associate location/provider labels & inputs
e20d44e 
fixes an accessibility issue with location/provider text fields
not having labels. Screenreaders would not be able to provide the context
of the input to the user.

The labels were present but they were not using the  correct input id
for their `for` attribute.

(No visual difference but users should be able to click on the labels)
@aliuk2012
[3155] Error summary link set focus on 1st radio
d986e40 
The error summary link should set focus on the first radio/checkbox in
a list or the invalid text field.
@aliuk2012
[3155] Improve usability of study type
d8083f0 
The user does not get the opportunity to select which type of studies they
are interested in as part of the initial search. We include all types
of study by default. The user can change the filter to narrow their search.

We have a validation that ensures at least one checkbox is ticked. However,
if the user decides to untick each option and submits the page, we provide
them with an error message stating they should pick at least one option
but both checkboxes are ticked by default so it maybe confusing to the user
as they did not tick each option and both checkboxes are already ticked.
They maybe confused as to how to correct this error as it seems to have
corrected itself.
@aliuk2012
[3155] Fix qualifications error summary link
04409e2 
error summary links should set focus on the first radio/checkbox
@aliuk2012 aliuk2012 force-pushed the 3155-fix-error-message-inconsistency-design-review branch from 86d493a to 04409e2 Compare March 25, 2020 08:26
@aliuk2012 aliuk2012 temporarily deployed to find-teacher-3155-fix-e-3pbmmm March 25, 2020 08:26 Inactive
@aliuk2012 aliuk2012 marked this pull request as ready for review March 25, 2020 08:30
@aliuk2012
Copy link
Contributor Author

[S] Fix error message inconsistency (design review)

dankmitchell
dankmitchell approved these changes Mar 25, 2020
View reviewed changes
Copy link
Contributor

@dankmitchell dankmitchell left a comment

Choose a reason for hiding this comment

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

LGTM

@dankmitchell dankmitchell merged commit e03ed2c into master Mar 25, 2020
@dankmitchell dankmitchell deleted the 3155-fix-error-message-inconsistency-design-review branch March 25, 2020 14:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dankmitchell dankmitchell dankmitchell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@aliuk2012 @dankmitchell