Skip to content
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

TAI-3083 : Introduce styling for check-your-answers summary content #823

Merged
merged 6 commits into from
Oct 16, 2017

Conversation

markhunter27
Copy link
Contributor

Introduction of new styling, derived from the GOV.UK design pattern here

Problem

A number of services have the same requirement when developing linear journeys, for an accessible and responsive confirmation page. Rather than define the relevant styling locally to services, we're adding to the assets-frontend library for consistent use across services.

Solution

The new sass partial we've introduced is taken directly from the GOV.UK service design manual example. No changes have been made.

Example Screenshot

The screenshot below is taken directly from the GOV.UK service design manual, and an illustration is now included in the generated component library.
image

@gavinwye
Copy link
Contributor

@markhunter27 Why does this need to be added to Assets Frontend for it to be used in your service?

We shouldn't be duplicating GOV.UK patterns in Assets frontend the aim is to remove as many things as possible from AF and only add things that are specific to HMRC.

@markhunter27
Copy link
Contributor Author

@gavinwye We could introduce the styling locally to our service (TAI in this case), but there seems to be a convention already that a number of GOV.UK patterns are present within Assets Frontend - for instance this PR #789
We're aware that multiple teams within the DDCN are in need of this pattern - so placing it within Assets Frontend will remove duplication.

@roblav
Copy link
Contributor

roblav commented Oct 16, 2017

@markhunter27 Team 9 also need to use this pattern in our service.
@gavinwye currently isn't this pattern available only as an example in the prototype kit, here https://govuk-prototype-kit.herokuapp.com/docs/examples/check-your-answers-page, which uses a custom scss file _check-your-answers.scss.
So as far as I'm aware the styling is not available in Gov Elements or anywhere else for us to make use of, please correct me if I'm wrong.

@rpowis
Copy link
Contributor

rpowis commented Oct 16, 2017

The new sass partial we've introduced is taken directly from the GOV.UK service design manual example. No changes have been made.

@markhunter27 Is that right approach? I'm aware of some improvements that could be made (esp. accessibility). This feels like a good opportunity to make them.

cc @adamliptrot-oc @philsherry

@gavinwye
Copy link
Contributor

@rpowis we could add the pattern in its current state so that teams can use it and stop duplicating code in their services.
Then make the necessary changes to make the pattern more accessible and propose these through the working group back to GOV.UK.

@adamliptrot-oc
Copy link
Contributor

I'd definitely feel happier if we didn't use this pattern as it stands (and I know @philsherry has strong views on it). However I do think we need check your answers to be updated in AF as everyone is currently rolling their own.

@rpowis
Copy link
Contributor

rpowis commented Oct 16, 2017

@philsherry created this one alphagov/govuk-prototype-kit#334.

Maybe we go with that here instead?

@markhunter27
Copy link
Contributor Author

@rpowis Would you propose introducing within AF, or submitting to GOV elements for consideration?
Just had a quick conversation around the desk with @philsherry and @roblav. As @adamliptrot-oc mentions, Phil has previously created an implementation for his project (AMLS) which addresses some visual spacing concerns and also considers accessibility.
Phils feeling is that his implementation may be useful to others, but may not be a one-size-fits-all. He suggested that one approach may be for both implementations to be present with AF - the GOV.UK approach we've adopted here, and his own. From discussion it sounds as though the GOV example may actually be a previous response to Phils implementation.

We'll need to raise a separate PR for Phils implementation, so given that we need to meet our current sprint commitment we're proposing to bundle the styling into our own service for now - while we reach agreement on which should be placed within AF.

@rpowis
Copy link
Contributor

rpowis commented Oct 16, 2017

Would you propose introducing within AF, or submitting to GOV elements for consideration?

Definitely in AF where we can improve it for all HMRC services before proposing it as an improvement to the GOV.UK version.

Phils feeling is that his implementation may be useful to others, but may not be a one-size-fits-all.

If that's the case then we should get this version in now and then improve it.

@adamliptrot-oc @philsherry @gavinwye If everyone's happy with that can you thumbs up this comment and I'll merge and release it.

@rpowis
Copy link
Contributor

rpowis commented Oct 16, 2017

@markhunter27 Can you rebase master in the meantime please?

@gavinwye
Copy link
Contributor

I'm happy with that approach.

However we are building a design system to bring consistency to all our services so whatever approach we arrive at, for an HMRC version of check your answers will need to work for all services on the platform.

@gavinwye
Copy link
Contributor

There is an issue in the design patterns backlog that this work can be done against hmrc/design-patterns#26

@rpowis rpowis merged commit 54a3df4 into hmrc:master Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants