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

Use formatted name for worldwide organisations #3029

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

brucebolt
Copy link
Member

@brucebolt brucebolt commented Jan 2, 2024

We are currently presenting the Worldwide Organisation's title in the logo. This means there are no line breaks in the places specified by the user.

Updating to use the formatted title, which the publisher would have expected since they entered this in Whitehall.

Screenshots

Page Current Rendering Changed Rendering
Worldwide Organisation Screenshot showing the logo on a worldwide organisation page with no line break Screenshot showing the logo on a worldwide organisation page with the line break in the correct place
Worldwide Office Screenshot showing the logo on a worldwide office page with no line break Screenshot showing the logo on a worldwide office page with the line break in the correct place
Worldwide Corporate Information Page Screenshot showing the logo on a worldwide corporate information page with no line break Screenshot showing the logo on a worldwide corporate information page with the line break in the correct place

Trello card

@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3029 January 2, 2024 10:46 Inactive
@brucebolt
Copy link
Member Author

Depends on alphagov/publishing-api#2587.

@brucebolt brucebolt force-pushed the use-ww-org-formatted-name branch from e81e925 to 1a23c25 Compare January 2, 2024 13:28
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3029 January 2, 2024 13:29 Inactive
@brucebolt brucebolt force-pushed the use-ww-org-formatted-name branch from 1a23c25 to 538bcda Compare January 2, 2024 13:43
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3029 January 2, 2024 13:43 Inactive
@brucebolt brucebolt force-pushed the use-ww-org-formatted-name branch from 538bcda to b950143 Compare January 2, 2024 14:26
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3029 January 2, 2024 14:26 Inactive
We are currently presenting the Worldwide Organisation's title in the
logo. This means there are no line breaks in the places specified by the
user.

Updating to use the formatted title, which the publisher would have
expected since they entered this in Whitehall.

This also applies the change to Worldwide Offices and Worldwide
Corporate Information Pages, which include the Worldwide Organisation's
logo in the header.
@brucebolt brucebolt force-pushed the use-ww-org-formatted-name branch from b950143 to bae6511 Compare January 3, 2024 09:40
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3029 January 3, 2024 09:40 Inactive
@brucebolt brucebolt marked this pull request as ready for review January 3, 2024 10:53
Copy link
Contributor

@jkempster34 jkempster34 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the screenshots

@@ -41,7 +41,8 @@ class WorldwideCorporateInformationPageTest < ActionDispatch::IntegrationTest
setup_and_visit_content_item("worldwide_corporate_information_page")

assert_has_component_organisation_logo
assert page.has_link? "British Embassy Manila", href: "/world/organisations/british-embassy-manila"
assert_has_component_title("British Embassy\nManila")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why this works with a \n and not a <br>😕

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's because we're matching on text here, rather than the raw HTML, so the line break has already been parsed.

@@ -41,7 +41,8 @@ class WorldwideCorporateInformationPageTest < ActionDispatch::IntegrationTest
setup_and_visit_content_item("worldwide_corporate_information_page")

assert_has_component_organisation_logo
assert page.has_link? "British Embassy Manila", href: "/world/organisations/british-embassy-manila"
assert_has_component_title("British Embassy\nManila")
assert page.has_link? "British EmbassyManila", href: "/world/organisations/british-embassy-manila"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shame this doesn't also work with the \n

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed. There seems to be some inconsistency in the way these matchers work.

@@ -42,7 +42,7 @@ def schema_name

presented = create_presenter(WorldwideCorporateInformationPagePresenter, content_item: without_sponsoring_organisations)

expected = { name: "British Embassy Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
expected = { name: "British Embassy<br/>Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
Copy link
Contributor

@hannako hannako Jan 3, 2024

Choose a reason for hiding this comment

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

I don't understand where this <br/> is coming from. I can see the title in the schema example...

Copy link
Member Author

Choose a reason for hiding this comment

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

It comes from the formatted_title.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh different field! OK gottit!

Copy link
Contributor

@hannako hannako left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks Bruce

@brucebolt brucebolt merged commit 6a31f32 into main Jan 3, 2024
12 checks passed
@brucebolt brucebolt deleted the use-ww-org-formatted-name branch January 3, 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