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

Add header #4289

Merged
merged 3 commits into from
Oct 17, 2024
Merged

Add header #4289

merged 3 commits into from
Oct 17, 2024

Conversation

JamesCGDS
Copy link
Contributor

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

This PR adds the landing page header. It is dependent on PR #4284 in the components gem being released.

Why

Trello card

@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4289 October 16, 2024 14:46 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, just a couple of things. Will wait to approve pending a BE review.

app/assets/stylesheets/views/_landing_page.scss Outdated Show resolved Hide resolved
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4289 October 16, 2024 15:35 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4289 October 17, 2024 07:46 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4289 October 17, 2024 09:04 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4289 October 17, 2024 12:57 Inactive
Copy link
Contributor

@KludgeKML KludgeKML left a comment

Choose a reason for hiding this comment

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

Backend approved.

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 looks good to me 👍

This keeps the breadcrumbs in the sub header with the dark background. Otherwise, the breadcrumbs render above the sub header on a white background. On a separate note, due to the current absence of a content item, I've used Hash.new as the content item to prevent the breadcrumb component from erroring. Additionally, due to the current absence of a content item, both sets of breadcrumbs will currently render; this will be resolved once we pass through the content item.
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4289 October 17, 2024 13:23 Inactive
@JamesCGDS JamesCGDS merged commit 736bf7b into main Oct 17, 2024
12 checks passed
@JamesCGDS JamesCGDS deleted the add_header branch October 17, 2024 13:29
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.

4 participants