Move manual show page to design system #2144
Conversation
mtaylorgds
force-pushed
the
consolidation-move-manual-show-page-to-design-system
branch
5 times, most recently
from
ed5395c to
2558a8b
Compare
mtaylorgds
changed the title
mtaylorgds
force-pushed
the
consolidation-move-manual-show-page-to-design-system
branch
2 times, most recently
from
b3039d4 to
94e852f
Compare
There was a problem hiding this comment.
A bit hard for me to review really given I haven't done a lot of frontend work - I'm not really sure how the publishing components should be used. In terms of breaking it up a bit I can see why it's tricky, it's all quite coupled together. The only way I can see to do it is a two-step migration where you create new erb files and scss files for some of the page components/helpers (possibly in a commit per component), then change the manual page itself and remove the old components. That might be two separate PRs, depending on how you want to organise it, or just several commits (but it should be possible to keep tests green in each of them). That's a lot of extra effort to go to, but I suspect other reviewers might find that more digestible.
| } | ||
| } | ||
|
|
||
| .app-view-summary__sidebar-actions .govuk-list--spaced li:last-child div + .broken-links-report { |
There was a problem hiding this comment.
It is unfortunate that this is necessary 😞
| margin-bottom: govuk-spacing(6); | ||
| background-color: govuk-colour("light-grey"); | ||
|
|
||
| .gem-c-button { |
There was a problem hiding this comment.
What's the advice on overriding design system component styles?
There was a problem hiding this comment.
Not sure, but it doesn't look great without doing this (it's also just the spacing around the component, rather than changing its styling per se).
app/helpers/application_helper.rb
Outdated
| row[:key] = if section.state == "draft" | ||
| tag.span("DRAFT", class: "govuk-tag govuk-tag--s govuk-tag--blue") << | ||
| tag.span(section.title, class: "govuk-!-static-margin-2") |
There was a problem hiding this comment.
This is a little bit hard to read, I would argue
There was a problem hiding this comment.
Fair, though I'm struggling to write anything that looks really nice for this. I've had a stab at rewriting a bit.
(If you're referring to the way the row[:key] is assigned the result of the conditional, that's how rubocop wants it to be.)
I'm open to suggestions for further improvements…
There was a problem hiding this comment.
If that's what rubocop wants, it's good enough for me 🙈
| </div> | ||
| </div> | ||
| <% end %> | ||
|
|
There was a problem hiding this comment.
Would it make any sense to move this out in to a separate commit? It doesn't seem directly related to the show page
There was a problem hiding this comment.
It's tricky, because the show page is the first existing page moving over to the design system. The show page uses flash messages, so it is what's driving the need for this being added to the layout template.
There was a problem hiding this comment.
Yes, fair enough. I suspected I wouldn't be able to offer much of any use on this.
mtaylorgds
force-pushed
the
consolidation-move-manual-show-page-to-design-system
branch
2 times, most recently
from
c6e127f to
7c08a05
Compare
Update the layout of the "show" view from using the legacy layout to use the design system one. This change significantly alters the layout of the page, but without changing any of the underlying functionality.
mtaylorgds
force-pushed
the
consolidation-move-manual-show-page-to-design-system
branch
from
7c08a05 to
0702a75
Compare
|
Closing as have noticed an issue with the link checker on legacy forms. |
What
Migrate the manual 'show' view to the design system layout.
Why
The entire Manuals Publisher app is being moved over to use the more modern design system and associated components. This change should also bring it more in line with Whitehall, reducing the usability friction for people who need to work across both apps.
Visuals
Before
After
Trello
Trello card
Follow these steps if you are doing a Rails upgrade.