-
-
Notifications
You must be signed in to change notification settings - Fork 400
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
feat: serve content without header and footer for mobile app #11245
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #11245 +/- ##
==========================================
+ Coverage 49.15% 49.34% +0.19%
==========================================
Files 79 79
Lines 22502 22504 +2
Branches 5386 5387 +1
==========================================
+ Hits 11061 11105 +44
+ Misses 10076 10038 -38
+ Partials 1365 1361 -4 ☔ View full report in Codecov by Sentry. |
/update_tests_results |
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.
Great idea.
I approve to avoid slowing you down but I have some complains still ;-)
lib/ProductOpener/Display.pm
Outdated
my $template = 'web/common/site_layout.tt.html'; | ||
# ?content_only=1 -> only the content, no header, footer, etc. | ||
if (($user_agent =~ /smoothie/) or (single_param('content_only'))) { | ||
$template = 'web/common/content_only.tt.html'; | ||
} |
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.
nitpick: I think it would be worth isolating in a get_site_layout method
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.
Can't we avoid a full rewrite of the content template ?
Isn't it possible to separate common parts in another template and include it in both ?
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's possible, but then it makes tons of small files.. I think it would be better to use [% IF ! content_only %] directives to remove what we don't want in fact.
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.
@alexgarel I think it's better that way: ebee79c
…-server into content-only
/update_tests_results |
Quality Gate passedIssues Measures |
🤖 I have created a release *beep* *boop* --- ## [2.55.0](v2.54.0...v2.55.0) (2025-01-17) ### Features * determine packaging components in contact with food ([#11238](#11238)) ([c5cda35](c5cda35)) * serve content without header and footer for mobile app ([#11245](#11245)) ([662a96b](662a96b)) ### Bug Fixes * Green-Score attributes/panels titles and subtitles ([#11244](#11244)) ([1eb84a5](1eb84a5)) * more positive messages for Nova 4 ([#11231](#11231)) ([79e63cd](79e63cd)) * Update fundraiser text to 2025 ([#11248](#11248)) ([b88f43e](b88f43e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This PR is for better display of content pages in the mobile app. It removes the header or the footer if: