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

[WIP] [DO NOT MERGE] Redesign No. 10 organisation page #3605

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

AshGDS
Copy link
Contributor

@AshGDS AshGDS commented May 1, 2024

⚠️ Please do not merge ⚠️

What

  • Redesign No. 10 organisation page

Why

Things for frontend to look at:

  • Latest news page
  • The image card is always large: true when a YouTube video is present, causing a formatting issue, so component change is needed

Things for backend to look at:

  • Latest news doesn't come with the right data for the image card component so ive hacked it for now.
    • Also I can't figure out how to remove the duplicate news item from the list to the right of it (doing .shift(), .drop() etc on the array doesn't work)
  • I can't change the colour of the contacts links as they're coming through from the backend, at app/presenters/organisations/contacts_presenter.rb line 68 (basically you need to remove brand__color if its the no 10 page) and I can't figure out how to do it as the @org.is_no_10? boolean doesn't work in that presenter
  • The follow links rendering relies on @what_we_do.has_share_links?, so maybe they can't remove the 'what we do' section from Whitehall?
  • Question around tests - they seem to be complaining that I haven't added loads of translations for the new strings I've added. Is there a way to make the tests ignore them? Or should we be getting translations for these new strings?

@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 1, 2024 15:28 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 1, 2024 15:30 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 1, 2024 15:39 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 1, 2024 15:54 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 1, 2024 16:00 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 1, 2024 16:02 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 10:38 Inactive
@AshGDS AshGDS self-assigned this May 2, 2024
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 13:48 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 14:21 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 14:40 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 14:50 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 14:59 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 15:04 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 15:10 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 15:23 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 2, 2024 15:46 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 3, 2024 08:51 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 3, 2024 11:00 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 3, 2024 11:32 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 7, 2024 07:29 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 7, 2024 07:32 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 7, 2024 08:45 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 7, 2024 13:01 Inactive
Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

Some minor comments, mainly noting the lack of tests in some areas, but I appreciate that not everything might be testable, or covered by other kinds of tests, so happy to discuss.

app/views/organisations/_videos.html.erb Outdated Show resolved Hide resolved
app/views/organisations/_videos.html.erb Show resolved Hide resolved
app/views/organisations/_latest_news.html.erb Show resolved Hide resolved
app/helpers/promotional_feature_helper.rb Show resolved Hide resolved
app/helpers/latest_news_helper.rb Show resolved Hide resolved
- add test for missing image reporting
- move promotional features around to match promotional
  features with video details
- add stubs for missing news items to test handling
  of unusual cases on integration
- Split promotional features methods into their own helper
- Add styles for videos partials links
- Videos partial
- Add the video title as a heading
- Use brand color for more videos link
- Use YouTube video URL as the YouTube link text
  To prevent duplication with the heading above it in no-js
  situations.
- Tests for helper
- Helper that grabs latest news from a search
- Also grabs content item for first latest news item to get image
- Partial for latest news
- Add default image fallback (No. 10 front door, since this is only
  used by No. 10 at the moment)
- tests for helper
- partial for small feature news items
- partial for side-by-side prmotional features
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 15, 2024 11:09 Inactive
@KludgeKML KludgeKML requested review from KludgeKML and removed request for KludgeKML May 15, 2024 12:07
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 16, 2024 12:25 Inactive
@govuk-ci govuk-ci temporarily deployed to collections-pr-3605 May 17, 2024 13:44 Inactive
Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

Frontend code looks good to me 👏

@andysellick andysellick changed the title [WIP] Redesign No. 10 organisation page [WIP] [DO NOT MERGE] Redesign No. 10 organisation page May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants