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

Remove subscribe app component #4535

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MartinJJones
Copy link
Contributor

@MartinJJones MartinJJones commented Dec 16, 2024

What

  • Remove the subscribe app component and replace it with a link
  • Remove the icon-calendar PNG images, they were only used by the subscribe component
  • Updated the GA4 tracking test for link to ICS files on the bank holidays page

Why

The subscribe component is not required in the application, the only difference between this component and a regular link is the inclusion of a calendar icon. The spacing and format of the icon is also inconsistent with other components where icons are used.

The text and style of the download will likely change in a future PR, but this will be dependant on translations and design changes.

Visual Changes

UK bank holidays

Before After
bank-holiday-before bank-holiday-after

https://govuk-frontend-app-pr-4535.herokuapp.com/bank-holidays

When do the clocks change?

Before After
clock-change-before clock-change-after

https://govuk-frontend-app-pr-4535.herokuapp.com/when-do-the-clocks-change

Trello card

@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4535 December 16, 2024 17:11 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4535 December 18, 2024 17:04 Inactive
The app level component is no longer required, it will be replaced with the button component from the govuk_publishing_components gem
The `icon-calendar` pngs can be deleted as they were only used by the `subscribe` component which will also be removed as part of this PR.
The `.app-c-subscribe` CSS class is no longer present following the removal of the subscribe app component.

The test has been updated to get all links than end with the `.ics` file extension and check they have the correct data attributes set.
@MartinJJones MartinJJones force-pushed the remove-subscribe-app-component branch from 1292a29 to 6d45ab8 Compare December 19, 2024 10:07
@MartinJJones MartinJJones changed the title [WIP] Remove subscribe app component Remove subscribe app component Dec 19, 2024
@MartinJJones MartinJJones marked this pull request as ready for review December 19, 2024 11:56
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.

2 participants