-
Notifications
You must be signed in to change notification settings - Fork 84
Improve application layouts #12430
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
base: main
Are you sure you want to change the base?
Improve application layouts #12430
Conversation
Luke-Oldenburg
left a comment
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.
Please use strict locals if passing variables to partials. Also, I think the names could have "shared" removed / be tweaked.
Luke-Oldenburg
left a comment
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.
It seems you have some unrelated changes.
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.
Pull request overview
This PR consolidates duplicate application layout code into shared partials, improving code maintainability by extracting common head tags, body suffix elements, and seasonal decorations into reusable components.
Key Changes:
- Extracted common head tags (meta tags, assets, console art) into
_head.html.erbpartial with configurable options - Consolidated seasonal decorations (snow, halloween elements) into
_seasonal.html.erbpartial - Created
_body_suffix.html.erbpartial for fullstory tracking, confetti effects, and feature flag styling
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
app/views/layouts/login.html.erb |
Replaced duplicated head content with render "layouts/head", consolidated seasonal decorations |
app/views/layouts/docs.html.erb |
Refactored to use shared head, seasonal, and body_suffix partials |
app/views/layouts/application.html.erb |
Migrated to shared partials for head, seasonal, and body suffix code |
app/views/layouts/admin.html.erb |
Updated to use shared head partial with admin-specific configuration |
app/views/layouts/_seasonal.html.erb |
New partial for seasonal decorations with configurable snow style |
app/views/layouts/_head.html.erb |
New partial consolidating common head tags with multiple configuration options |
app/views/layouts/_body_suffix.html.erb |
New partial for fullstory, confetti, and feature flag styling |
app/views/layouts/_shared_seasonal.html.erb |
Duplicate unused partial that should be removed |
app/views/layouts/_shared_head.html.erb |
Duplicate unused partial that should be removed |
app/views/layouts/_shared_body_suffix.html.erb |
Duplicate unused partial that should be removed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Luke Oldenburg <87272260+Luke-Oldenburg@users.noreply.github.com>
Co-authored-by: Luke Oldenburg <87272260+Luke-Oldenburg@users.noreply.github.com>
Luke-Oldenburg
left a comment
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.
Dark theme isn't set on refresh.
|
fixed! |
Luke-Oldenburg
left a comment
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

Previously blocked by UI3, this PR improves HCB's code quality by consolidating duplicate application layout file code.