[19.0][IMP] web_pwa_customize: apple-touch-icon + settings visibility - #3663
Open
LadyHwesta wants to merge 2 commits into
Open
LadyHwesta wants to merge 2 commits into
LadyHwesta wants to merge 2 commits into
Conversation
web_pwa_customize only ever touched /web/manifest.webmanifest (Chrome, Edge, Android). iOS Safari's "Add to Home Screen" reads the apple-touch-icon link tag instead, which this module never set, so an icon configured here never showed up there at all. Adds a /web_pwa_customize/apple_touch_icon route that finds the best available configured icon (preferring the 192x192 PNG variant, falling through the other generated sizes, then an SVG upload, then Odoo's own artwork if nothing is configured) and points the apple-touch-icon tag at it via a one-attribute template inherit on web.webclient_bootstrap. Also drops the pwa_settings block's base.group_no_one restriction - this module's own fields (and core's pre-existing web.web_app_name, which nothing else surfaces either) were only ever reachable with developer mode on. General Settings already requires admin access, so this doesn't expose anything that wasn't already admin-only. 5 new tests, all passing alongside the 3 existing ones.
Contributor
|
Hi @victoralmau, |
…brittle CI failed 3 of the 5 new tests: Werkzeug renders a redirect's Location header as a bare path in some environments and as an absolute URL in others (both are valid per RFC 7231) - the tests asserted exact equality against a bare path, which only happened to match locally. Assert on the path suffix instead, so either form passes.
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
2 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's missing today
web_pwa_customizelets you configure a short name, colors, and icon for Odoo's "Install app" prompt - but only for/web/manifest.webmanifest, which Chrome, Edge, and Android read. iOS Safari's "Add to Home Screen" reads a separateapple-touch-iconlink tag instead, which this module never touches - so an icon configured here never shows up there at all.Its own settings also inherit an existing restriction: the
pwa_settingsblock (which also holds core's ownweb.web_app_namefield, itself not surfaced anywhere else) is hidden behindbase.group_no_one, i.e. invisible unless developer mode is on.What this PR does
/web_pwa_customize/apple_touch_iconroute that finds the best available configured icon - preferring the 192x192 PNG variant this module already generates, falling through the other sizes, then an SVG upload, then Odoo's own stock artwork if nothing is configured - and points theapple-touch-icontag at it via a one-attribute template inherit onweb.webclient_bootstrap.pwa_settingsblock'sgroups="base.group_no_one"restriction. General Settings itself already requires admin access, so this doesn't expose anything that wasn't already admin-only - it just stops requiring developer mode on top of that.No new dependencies, no changes to how the icon/colors are stored - purely additive on top of the existing
res.config.settingsfields andir.attachment-based icon storage.Testing
5 new tests (apple-touch-icon fallback with nothing configured, redirect to a configured PNG, redirect to a configured SVG, the link tag itself, and settings-block visibility for a plain admin without
base.group_no_one), all green alongside the 3 existing ones. Ran the full pre-commit suite (ruff, prettier, pylint-odoo, README regeneration) clean.Happy to adjust the approach if maintainers would rather see this split differently (e.g. a separate module) or handled another way.