Header sidebar content improvements#5648
Merged
Merged
Conversation
- Allows derived classes to set @content_item_presenter to their subclass of ContentItemPresenter and use it in views in the same way content_item is. - content_item is already added as a helper method in ApplicationController, so we don't need to add it here.
…m_presenter - TopicalEvent controller now has access to the base attribute, so doesn't need to define it itself. - TopicalEvent about view update to use the new variable
…senter - Update controller to use new attribute. - Update show page to use attribute instead of @presenter
- breadcrumbs becomes the default way to ask a presenter for a list of breadcrumbs. If there's nothing in the array, no breadcrumb components should be displayed. - Because this is going to only affect header_sidebar_content views at the moment, it's a limited change at the moment - We also add a default_breadcrumbs method which presenters can use to get the standard home.
- It is a content item presenter, because it has a content item, so we can get the default behaviour (including the breadcrumbs)
KludgeKML
force-pushed
the
header-sidebar-content-improvements
branch
from
f4087f4 to
e0fd39e
Compare
- Since we want everything that uses this layout to either have no breadcrumbs or have them with this exact margin/behaviour, we move the options into the layout. This is also nicer, unless it's saving us a lot of work we _should_ have the component options in the component call. - The only things using this layout at the moment are field of operation and topical event about page, and they support the breadcrumbs method in their presenter (default or overridden, respectively), so we don't need to put in any guards about whether the presenter exists or provides the method. - Now that both items are using simple breadcrumbs, we can drop the layout's potential call to contextual breadcrumbs, which was previously driving the breadcrumbs for the field_of_operations content items. - We also don't copy across bottom_margin: 5 from the about page presenter, instead using the default. This keeps good current appearance on the fields-of-operation page, and we can worry about the embedded about pages when they start to be tested from Whitehall.
KludgeKML
force-pushed
the
header-sidebar-content-improvements
branch
from
e0fd39e to
0a597a3
Compare
andysellick
reviewed
Jul 8, 2026
andysellick
left a comment
andysellick
left a comment
Contributor
There was a problem hiding this comment.
All looks sensible to me, but I'm not an expert. I like the idea of moving component options closer to the component in particular.
sairamya93
approved these changes
Jul 8, 2026
sairamya93
left a comment
sairamya93
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me too!
deborahchua
approved these changes
Jul 8, 2026
deborahchua
left a comment
deborahchua
left a comment
Contributor
There was a problem hiding this comment.
This is a nice solution!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Improve handling of breadcrumbs in header/sidebar/content layout.
Why
By focusing on the two things currently rendered by this layout, we can bring in incremental improvements. At the moment both of the routes handle their breadcrumbs independently. By moving breadcrumbs into the presenter and making a standard presenter variable that layouts will always have access to, we can simplify breadcrumb handling in the individual models (they just provide a breadcrumbs method, or not if they want the default Home breadcrumb), and in the layout (it can rely on a presenter with a breadcrumb method being there to determine whether or not it shows a breadcrumb, and the layout options of bottom_margin / collapse_on_mobile are defined in the layout, close to the component, rather than in a presenter somewhere), ensuring that we don't allow a proliferation of styles on this layout.
Visual changes
None, although we will have to review how about pages look when topical events with embedded about pages are tested coming out of Whitehall.
Reviewer notes:
Check:
... against its production version.