-
Notifications
You must be signed in to change notification settings - Fork 43
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
Conversation
c8767e8
to
a1738ed
Compare
e007320
to
cd4498e
Compare
@@ -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| %> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have implemented the flash messages as part of the actual page content itself.
dd8489a
to
3100f1c
Compare
6555561
to
469c123
Compare
- 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
469c123
to
dc24c16
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What
design_system_downtime_pages
to the FlipFlop config.legacy_
prefix to them.Why
As part of the work to transition Publisher to the Design System from Bootstrap.
Visual Differences (behind a feature flag toggle)
Before
After
Relevant Trello Card
Follow these steps if you are doing a Rails upgrade.