-
Notifications
You must be signed in to change notification settings - Fork 187
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
Update latest documents, publications and announcements template #6252
Update latest documents, publications and announcements template #6252
Conversation
8098614
to
5fe1e11
Compare
e1914d4
to
25be11b
Compare
25be11b
to
785a2cc
Compare
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.
Could some screenshots be included on the PR for ref?
031d11c
to
206ea86
Compare
e8186d9
to
402ab8f
Compare
c0bac61
to
08a9c96
Compare
I'm not convinced the Announcements pages works as it should however this is out of scope this PR- #6306 |
08a9c96
to
fd36f90
Compare
I think this looks Ok, I think there's some more long term considerations but just needs a rebase. |
e0838ef
to
f9a7e83
Compare
Hey @maxgds neither of us have auth to merge this one on Whitehall - give a look, if you're happy merge? |
app/views/latest/index.html.erb
Outdated
}, | ||
] | ||
} %> | ||
<% else subject.is_a? WorldLocation %> |
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.
what's going on here - is this an "else" or "elsif"?
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 might be wrong... but I think else
if correct? The structure of this block seems to be -
<% if subject.is_a? Organisation %>...
<% elsif subject.is_a? TopicalEvent %>...
<% else subject.is_a? WorldLocation %>...
<% end %>
So, it looks like the breadcrumb changes based on the subject, since it's a shared page, with "World Location" the default subject.
<div class="govuk-width-container"> | ||
<div class="govuk-grid-row"> | ||
<div class="govuk-grid-column-two-thirds"> | ||
<% if subject.is_a? Organisation %> | ||
<%= render "govuk_publishing_components/components/breadcrumbs", { |
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 could be tidier if we built the breadcrumbs array in the if and called the component to pass it to later - but it's beyond the scope of what's needed here.
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.
Okay, yep that would be better. I'll add a backlog ticket.
f9a7e83
to
31ca46b
Compare
…-template-and-Publications-locale" This reverts commit 2007f9a.
What
https://trello.com/c/th7Nw5lU/623-update-latest-from-world-locations-template-and-locale-publications
Change apply to the following pages:
Changes include:
h3
which breaks the document outline on these pagesselect_tag
in https://github.com/alphagov/whitehall/pull/6252/files#diff-fd350a6c20d48d3a4b48e3e0187e47fa07a0f05d60f68830ce60810cebd44106L13 with "select" componentWhy
Visual changes
The feeds component is in a shared partial, which is used by the topical events landing pages e.g. https://www.gov.uk/government/topical-events/2022-events-platinum-jubilee-commonwealth-games-festival-uk, so have included screenshots for this page too as they are affected by some of the changes above.