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

Initial work for Downtime Index page transition to Design System #2027

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

patrickpatrickpatrick
Copy link
Contributor

@patrickpatrickpatrick patrickpatrickpatrick commented Jan 29, 2024

What

  • Add new feature design_system_downtime_pages to the FlipFlop config.
  • Update the routes with new constraint.
  • Duplicate the controllers, views and test of Downtimes and append the legacy_ prefix to them.
  • Add new test in the routes integration test for the new feature.
  • Add new test for legacy views in integration for Downtimes.
  • Make the new Downtimes index page use the Design System layout.
  • Add flash back to the Design System layout for integration tests (and also for future work to flash success/failure messages).

Why

As part of the work to transition Publisher to the Design System from Bootstrap.

Visual Differences (behind a feature flag toggle)

Before

Screenshot 2024-01-29 at 16 16 37

After

Screenshot 2024-01-29 at 16 15 42

Relevant Trello Card

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

Follow these steps if you are doing a Rails upgrade.

@patrickpatrickpatrick patrickpatrickpatrick changed the title initial work for downtime Initial work for Downtime transition to Design System Jan 29, 2024
@patrickpatrickpatrick patrickpatrickpatrick force-pushed the flipflop-for-downtime branch 8 times, most recently from c8767e8 to a1738ed Compare January 30, 2024 16:30
@patrickpatrickpatrick patrickpatrickpatrick changed the title Initial work for Downtime transition to Design System Initial work for Downtime Index page transition to Design System Jan 30, 2024
@patrickpatrickpatrick patrickpatrickpatrick force-pushed the flipflop-for-downtime branch 4 times, most recently from e007320 to cd4498e Compare January 30, 2024 16:52
@@ -18,6 +18,19 @@

<div class="govuk-width-container">
<main class="govuk-main-wrapper" id="main-content" role="main">
<% [:success, :info, :warning, :danger, :notice, :alert].select { |k| flash[k].present? }.each do |k| %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be using the design system components (Error Alert, Success Alert, Notice), rather than copying this pattern from the legacy layout template?

Copy link
Contributor Author

@patrickpatrickpatrick patrickpatrickpatrick Jan 31, 2024

Choose a reason for hiding this comment

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

I thought that implementing the design system component would be a separate task in of itself. I think the flash message probably shouldn't have been removed initially or should have done part as part of the layout card, which is why I'm putting it back for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we need a team discussion around this, as I would expect that without using the components from the design system, the look of the flash messages won't be acceptable for releasing these pages, in which case we need to get a story for doing that done ASAP.

I think we probably need a discussion around how we're going to release pages (i.e. turn the default state of the toggle to on) anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

- Add new feature `design_system_downtime_index_page` to the FlipFlop config
- Update the routes with new constraint
- Duplicate the controllers, views and test of Downtimes and append the `legacy_` prefix to them
- Add new test in the routes integration test for the new feature
- Create legacy integration Downtimes test
- Make the new Downtimes index page use the Design System layout
- Add flash back to the Design System layout for integration tests
Copy link
Contributor

@cynthia-anya cynthia-anya left a comment

Choose a reason for hiding this comment

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

LGTM

@patrickpatrickpatrick patrickpatrickpatrick merged commit 8a8b315 into main Feb 1, 2024
13 checks passed
@patrickpatrickpatrick patrickpatrickpatrick deleted the flipflop-for-downtime branch February 1, 2024 16:22
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.

3 participants