-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add sections to routes wizard completed page #4490
Add sections to routes wizard completed page #4490
Conversation
d788cfa
to
d4fbd6a
Compare
Co-authored-by: Gemma Dallman <87642394+gemmadallmandfe@users.noreply.github.com>
<h2 class="heading--box-blue">Next steps</h2> | ||
|
||
<%= render CallsToAction::SimpleComponent.new(icon: "icon-arrow-black") do %> | ||
<h2 class="call-to-action__heading">Get personalised guidance</h2> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be an h3 @spencerldixon as it is under an h2 of Next steps
<div class="col-space-l-top"> | ||
<% if non_uk? %> | ||
<%= render Content::QuoteWithImageComponent.new( | ||
title: "Teaching as a non-uk...", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to have a heading level attributed to it @spencerldixon
I think it should be an h3 as it comes under the h2 of Next steps
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, looking in production, this component does not have a heading eg on https://getintoteaching.education.gov.uk/life-as-a-teacher/teaching-as-a-career/career-progression so I'm presuming we would need to look at this separately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed QuoteWithImageComponent to render a h3 by default, but I've set this as an option, so we can always override this with another value if needed elsewhere
) %> | ||
<% else %> | ||
<%= render Content::QuoteWithImageComponent.new( | ||
title: "Teacher training stories", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to have a heading level attributed to it @spencerldixon
I think it should be an h3 as it comes under the h2 of Next steps
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
…ltsBoxComponent to render a h3
Review app deployed to https://get-into-teaching-app-review-4490.test.teacherservices.cloud |
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the headers @spencerldixon
Happy for this to be merged into the main branch!
Trello card
https://trello.com/c/kv57CkUJ/7127-add-other-content-and-elements-to-routes-wizard-results-page?filter=member:spencerldixon
Context
We want to build the three question pages and render the answers on a results page
As well as the results (covered in separate tickets) we want to add other design elements / content to the page
Changes proposed in this pull request
Guidance to review