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

Reorder number 10 org page #3795

Merged
merged 1 commit into from
Dec 5, 2024
Merged

Reorder number 10 org page #3795

merged 1 commit into from
Dec 5, 2024

Conversation

AshGDS
Copy link
Contributor

@AshGDS AshGDS commented Oct 15, 2024

What / Why

Screenshots

image

@govuk-ci govuk-ci temporarily deployed to collections-pr-3795 October 15, 2024 11:45 Inactive
@AshGDS AshGDS changed the title Reorder number 10 org page [DO NOT MERGE] Reorder number 10 org page Oct 15, 2024
@AshGDS AshGDS force-pushed the redesign-org-page branch from 51fcc25 to 15b14a5 Compare October 16, 2024 10:34
@govuk-ci govuk-ci temporarily deployed to collections-pr-3795 October 16, 2024 10:34 Inactive
@AshGDS AshGDS force-pushed the redesign-org-page branch from 15b14a5 to 25f982c Compare October 16, 2024 10:40
@govuk-ci govuk-ci temporarily deployed to collections-pr-3795 October 16, 2024 10:41 Inactive
@AshGDS AshGDS force-pushed the redesign-org-page branch from 25f982c to 29dd040 Compare October 16, 2024 11:03
@govuk-ci govuk-ci temporarily deployed to collections-pr-3795 October 16, 2024 11:03 Inactive
Copy link
Contributor

@leenagupte leenagupte left a comment

Choose a reason for hiding this comment

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

One minor comment about array manipulation, otherwise it looks good to me.

@govuk-ci govuk-ci temporarily deployed to collections-pr-3795 October 17, 2024 10:50 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.

LGTM!

@@ -17,7 +17,14 @@
} %>
</div>
</div>

<%
@documents.first_featured_news[:context][:text] = "Featured"
Copy link
Contributor

Choose a reason for hiding this comment

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

You could theoretically get this from the localisation files, but don't let that be a blocker.

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 👍

@AshGDS AshGDS force-pushed the redesign-org-page branch 2 times, most recently from 4ce02d9 to 24d9f6b Compare December 4, 2024 15:21
@AshGDS AshGDS force-pushed the redesign-org-page branch from 24d9f6b to be1dbd9 Compare December 5, 2024 10:14
@AshGDS AshGDS changed the title [DO NOT MERGE] Reorder number 10 org page Reorder number 10 org page Dec 5, 2024
@AshGDS AshGDS merged commit 0cf9e59 into main Dec 5, 2024
14 checks passed
@AshGDS AshGDS deleted the redesign-org-page branch December 5, 2024 11:12
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.

5 participants