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

Update latest documents, publications and announcements template #6252

Merged

Conversation

jon-kirwan
Copy link
Contributor

@jon-kirwan jon-kirwan commented Aug 26, 2021

What

https://trello.com/c/th7Nw5lU/623-update-latest-from-world-locations-template-and-locale-publications

Change apply to the following pages:

Changes include:

Why

  • Consistent look with other pages
  • Use design system styles and component guide components
  • Fix document outline issues

Visual changes

Before After
## Anything else

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.

@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch from 8098614 to 5fe1e11 Compare August 26, 2021 16:55
@jon-kirwan jon-kirwan changed the title Update World Locations template Update World Locations and Publications template Aug 26, 2021
@jon-kirwan jon-kirwan changed the title Update World Locations and Publications template Update world locations and publications template Aug 26, 2021
@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch 5 times, most recently from e1914d4 to 25be11b Compare August 27, 2021 16:44
@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch from 25be11b to 785a2cc Compare September 6, 2021 11:37
@jon-kirwan jon-kirwan marked this pull request as draft September 6, 2021 13:39
@jon-kirwan jon-kirwan removed the request for review from chris-gds September 6, 2021 13:40
Copy link
Contributor

@chris-gds chris-gds left a 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?

app/views/announcements/index.html.erb Outdated Show resolved Hide resolved
app/views/documents/_filter_form.html.erb Outdated Show resolved Hide resolved
@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch 17 times, most recently from 031d11c to 206ea86 Compare September 7, 2021 17:19
@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch 4 times, most recently from e8186d9 to 402ab8f Compare September 15, 2021 20:22
@jon-kirwan jon-kirwan marked this pull request as ready for review September 16, 2021 08:52
@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch 3 times, most recently from c0bac61 to 08a9c96 Compare September 16, 2021 16:27
@jon-kirwan jon-kirwan changed the title Update world locations and publications template Update world location (latest), publications and announcements template Sep 23, 2021
@jon-kirwan jon-kirwan changed the title Update world location (latest), publications and announcements template Update latest documents, publications and announcements template Sep 27, 2021
@chris-gds
Copy link
Contributor

I'm not convinced the Announcements pages works as it should however this is out of scope this PR- #6306

@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch from 08a9c96 to fd36f90 Compare September 29, 2021 13:25
@chris-gds
Copy link
Contributor

I think this looks Ok, I think there's some more long term considerations but just needs a rebase.

@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch 2 times, most recently from e0838ef to f9a7e83 Compare October 5, 2021 16:02
@chris-gds
Copy link
Contributor

chris-gds commented Oct 6, 2021

Hey @maxgds neither of us have auth to merge this one on Whitehall - give a look, if you're happy merge?

},
]
} %>
<% else subject.is_a? WorldLocation %>
Copy link
Contributor

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"?

Copy link
Contributor Author

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", {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@jon-kirwan jon-kirwan force-pushed the Update-World-Locations-template-and-Publications-locale branch from f9a7e83 to 31ca46b Compare October 6, 2021 15:05
@maxgds maxgds merged commit 2007f9a into main Oct 6, 2021
@maxgds maxgds deleted the Update-World-Locations-template-and-Publications-locale branch October 6, 2021 15:22
chris-gds pushed a commit that referenced this pull request Oct 8, 2021
…-template-and-Publications-locale"

This reverts commit 2007f9a.
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.

None yet

3 participants