Update PWA styling with dark theme and mobile safe-area support #298
Conversation
Updated .gitattributes to ensure consistent LF line endings for bin, .sh, and .rb files to prevent shebang issues on Unix systems. Also removed trailing whitespace in bin/dev.
Updated dialog and layout components to use safe area insets for better mobile support. Changed PWA manifest and meta theme colors to dark for improved appearance on iOS and Android. Added more icon sizes and details to webmanifest for better device compatibility. Enhanced HTML structure to use dynamic viewport height and background color.
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/views/layouts/shared/_htmldoc.html.erb (1)
16-16: Consider if duplicate bg-surface is necessary.Both the
htmltag (line 10) andbodytag (line 16) now havebg-surface. Sincebodytypically covers the entirehtmlelement, thebg-surfaceonhtmlmay be redundant unless there are edge cases where body doesn't fill the viewport.If no visual gaps exist where html background shows through, remove
bg-surfacefrom line 10:- class="h-dvh text-primary overflow-hidden font-sans bg-surface <%= @os %>"> + class="h-dvh text-primary overflow-hidden font-sans <%= @os %>">public/site.webmanifest (2)
26-26: Consider "standalone" for better user experience.The "fullscreen" display mode removes all browser UI, which can be overly restrictive. The "standalone" mode is typically preferred for PWAs as it provides an app-like experience while maintaining essential system UI elements like the status bar.
Apply this diff if you'd like to use standalone mode:
- "display": "fullscreen", + "display": "standalone",
27-27: Consider whether orientation lock is necessary.Locking the orientation to "portrait-primary" prevents landscape usage, which may frustrate users who prefer or need to use the app in landscape mode. Unless there's a specific reason for this restriction, consider allowing natural device rotation.
If rotation should be allowed, you can simply remove this property:
- "display": "fullscreen", - "orientation": "portrait-primary" + "display": "fullscreen"Or keep it flexible by removing just the orientation:
"display": "fullscreen", - "orientation": "portrait-primary"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.gitattributes(1 hunks)app/components/DS/dialog.html.erb(1 hunks)app/components/DS/dialog.rb(1 hunks)app/views/layouts/application.html.erb(2 hunks)app/views/layouts/shared/_head.html.erb(1 hunks)app/views/layouts/shared/_htmldoc.html.erb(1 hunks)app/views/pwa/manifest.json.erb(1 hunks)bin/dev(1 hunks)public/site.webmanifest(2 hunks)
🧰 Additional context used
📓 Path-based instructions (17)
app/views/**/*.erb
📄 CodeRabbit inference engine (AGENTS.md)
app/views/**/*.erb: Keep heavy logic out of ERB views; prefer helpers/components instead
ERB templates are linted by erb-lint per .erb_lint.ymlAlways use the icon helper (icon) for icons; never call lucide_icon directly
Files:
app/views/layouts/shared/_head.html.erbapp/views/pwa/manifest.json.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
app/views/**/*.html.*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.html.*: Use partials only for simple, context-specific, mostly static HTML content.
Prefer semantic HTML; use Turbo Frames where possible instead of client-side solutions.
Use query params for state, not localStorage or sessionStorage.
Always perform server-side formatting for currencies, numbers, and dates.
Files:
app/views/layouts/shared/_head.html.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
**/*.{html,erb,slim,js,jsx,ts,tsx,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use functional design tokens (e.g., text-primary, bg-container) from the design system; do not use raw colors or ad-hoc classes.
Files:
app/views/layouts/shared/_head.html.erbapp/components/DS/dialog.html.erbapp/views/pwa/manifest.json.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
app/views/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/views/**/*.html.erb: Prefer native HTML elements (e.g., ,) over JS-based components
Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Prefer native client-side form validation when possible
Files:
app/views/layouts/shared/_head.html.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
app/views/layouts/**/*.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Be mindful of performance in global layouts (e.g., avoid loading large data payloads)
Files:
app/views/layouts/shared/_head.html.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/views/layouts/shared/_head.html.erbapp/views/pwa/manifest.json.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
{app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controller lifecycle (e.g., avoid addEventListener in connect); controllers should just respond to actions
Files:
app/views/layouts/shared/_head.html.erbapp/components/DS/dialog.html.erbapp/views/pwa/manifest.json.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
{app/components/**/*.{js,ts,erb},app/views/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Component-scoped Stimulus controllers in app/components must be used only within their component views, not in app/views
Files:
app/views/layouts/shared/_head.html.erbapp/components/DS/dialog.html.erbapp/views/pwa/manifest.json.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Style UI using TailwindCSS v4.x with the custom design system defined in app/assets/tailwind/maybe-design-system.css
Always start by referencing app/assets/tailwind/maybe-design-system.css to understand available primitives, functional tokens, and component tokens before styling
Prefer functional tokens from the design system over raw Tailwind values (e.g., use text-primary, bg-container, border border-primary instead of text-white, bg-white, border-gray-200)
Files:
app/views/layouts/shared/_head.html.erbapp/views/pwa/manifest.json.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/layouts/shared/_head.html.erbapp/views/pwa/manifest.json.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
{app/views,app/components}/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
{app/views,app/components}/**/*.html.erb: Keep domain logic out of ERB templates; compute values in component/controller code and reference them in the template
Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Pass data from Rails to Stimulus via data-*-value attributes, not inline JavaScript
Files:
app/views/layouts/shared/_head.html.erbapp/components/DS/dialog.html.erbapp/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Name partials with a leading underscore (e.g., _trend_change.html.erb, _form_errors.html.erb)
Files:
app/views/layouts/shared/_head.html.erbapp/views/layouts/shared/_htmldoc.html.erb
app/components/**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/components/**/*: Prefer ViewComponents for reusable or complex UI components; keep domain logic out of view templates.
Logic belongs in component files, not in component template (*.html.erb, *.html.slim) files.
Files:
app/components/DS/dialog.html.erbapp/components/DS/dialog.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/components/DS/dialog.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/components/DS/dialog.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
app/components/DS/dialog.rb
app/views/layouts/application.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use Turbo frames in the application layout to load controller actions as demonstrated
Files:
app/views/layouts/application.html.erb
🧠 Learnings (15)
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/**/*.html.erb : Prefer native HTML elements (e.g., <dialog>, <details><summary>) over JS-based components
Applied to files:
app/components/DS/dialog.html.erbapp/views/layouts/shared/_htmldoc.html.erbapp/components/DS/dialog.rbapp/views/layouts/application.html.erb
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to app/{views,components}/**/*.html.erb : In ERB views, wire Stimulus interactions declaratively using data-action to call controller methods
Applied to files:
app/components/DS/dialog.html.erb
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb} : Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controller lifecycle (e.g., avoid addEventListener in connect); controllers should just respond to actions
Applied to files:
app/components/DS/dialog.html.erb
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to **/*.{rb,js,jsx,ts,tsx} : Make changes atomic, testable, and explain their impact briefly in code suggestions.
Applied to files:
.gitattributes
📚 Learning: 2025-09-16T13:17:53.155Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:17:53.155Z
Learning: Applies to app/views/**/*.erb : ERB templates are linted by erb-lint per .erb_lint.yml
Applied to files:
.gitattributes
📚 Learning: 2025-09-16T13:17:53.155Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:17:53.155Z
Learning: Run the full test suite locally (bin/rails test) and ensure it is green before pushing
Applied to files:
.gitattributes
📚 Learning: 2025-09-16T13:17:53.155Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:17:53.155Z
Learning: Applies to **/*.rb : Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Applied to files:
.gitattributes
📚 Learning: 2025-08-22T18:30:26.758Z
Learnt from: jjmata
Repo: we-promise/sure PR: 116
File: app/views/family_exports/_list.html.erb:16-28
Timestamp: 2025-08-22T18:30:26.758Z
Learning: In the family exports UI (app/views/family_exports/_list.html.erb), raw color utilities like bg-green-500/5, text-green-500, bg-red-500/5, text-red-500 are preferred over design system tokens for status indicators to maintain UI legibility and consistency with the existing import UI patterns. The user has indicated that design token replacements would make the UI illegible in this context.
Applied to files:
app/views/pwa/manifest.json.erb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/**/*.html.erb : Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Applied to files:
app/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to **/*.{html,erb,slim,js,jsx,ts,tsx,css,scss} : Always use functional design tokens (e.g., text-primary, bg-container) from the design system; do not use raw colors or ad-hoc classes.
Applied to files:
app/views/layouts/shared/_htmldoc.html.erb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/{views,components}/**/*.html.erb : Keep domain logic out of ERB templates; compute classes and logic in component/Ruby code, not inline in the template
Applied to files:
app/views/layouts/shared/_htmldoc.html.erb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/views/**/*.html.erb : Use ViewComponents when elements have complex logic/styling, are reused across contexts, need variants/slots/accessibility, or require Stimulus interactivity
Applied to files:
app/views/layouts/shared/_htmldoc.html.erb
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-09-23T22:23:17.441Z
Learning: Applies to {app/views/**,app/helpers/**} : Always generate semantic HTML
Applied to files:
app/views/layouts/shared/_htmldoc.html.erb
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to app/views/**/*.html.* : Prefer semantic HTML; use Turbo Frames where possible instead of client-side solutions.
Applied to files:
app/views/layouts/shared/_htmldoc.html.erb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/layouts/application.html.erb : Use Turbo frames in the application layout to load controller actions as demonstrated
Applied to files:
app/views/layouts/shared/_htmldoc.html.erb
🔇 Additional comments (12)
bin/dev (1)
6-6: LGTM - Improves readability.The added blank line improves visual separation between the foreman installation and the rbenv rehash logic, aligning with the line-ending consistency improvements in this PR.
app/components/DS/dialog.rb (1)
93-93: LGTM - Safe-area insets properly implemented.The safe-area insets handle device notches correctly. Note that both the container (this file) and the actions section (dialog.html.erb) apply bottom safe-area padding, which appears intentional for ensuring proper spacing throughout the dialog.
.gitattributes (1)
11-14: LGTM - Proper cross-platform line ending handling.These attributes ensure consistent LF line endings for scripts and Ruby files, preventing shebang issues when the repository is cloned on Windows. The explanatory comment clearly documents the intent.
app/components/DS/dialog.html.erb (1)
29-29: Verify safe-area bottom padding isn't duplicated.The parent container in
app/components/DS/dialog.rb(line 93) appliespb-[env(safe-area-inset-bottom)], and this actions div addspb-[calc(1rem+env(safe-area-inset-bottom))]. This results in double application of the safe-area inset, potentially creating excessive bottom spacing.If the intent is to have both the container's safe-area padding AND the actions' 1rem padding, the current implementation is correct. Otherwise, consider using just
pb-4here since the parent already handles safe-area.app/views/layouts/shared/_htmldoc.html.erb (1)
10-10: LGTM - Proper viewport height for PWA.Switching from
h-fulltoh-dvhcorrectly handles mobile browser chrome that dynamically appears/disappears. Usingbg-surfacedesign token follows the coding guidelines.app/views/layouts/application.html.erb (2)
161-161: LGTM - Fixed mobile navigation properly implemented.Converting the mobile nav to
fixedpositioning withbottom-0 left-0 w-fullis the correct approach for persistent bottom navigation in PWAs. The safe-area inset padding and z-index (z-10) appropriately handle device notches and layering.
115-116: Define explicit nav height to eliminate magic number coupling.The
3.75rembottom padding does currently match the mobile nav's rendered height (based onw-8 h-8icon +text-[11px]text + spacing), so the value is correct. However, the nav itself has no explicit height defined, making it fragile: any change to icon sizing, font size, or spacing in_nav_item.html.erbwould break alignment without requiring updates to line 116.To make this maintainable, add an explicit height class to the nav at line 161 and reference a shared constant:
<%= tag.nav class: "lg:hidden h-15 bg-surface fixed bottom-0 left-0 w-full shrink-0 z-10 pb-[env(safe-area-inset-bottom)] border-t border-tertiary flex justify-around" do %>Or define a Tailwind token in
maybe-design-system.csslike--spacing-mobile-nav: 3.75remand update the main padding to use it. This ensures both the nav and content padding stay synchronized if nav styling changes.app/views/layouts/shared/_head.html.erb (2)
22-22: LGTM - Proper color scheme declaration.The
color-scheme: dark lightmeta tag correctly informs browsers that the app supports both color schemes with dark as the preference, which aligns with the PWA theming updates.
24-26: Raw color usage acceptable for meta tags.Using the raw color value
#0B0B0Bin meta tags is necessary and appropriate here, as meta tags cannot reference CSS design tokens. This is an exception to the general guideline of using design tokens.app/views/pwa/manifest.json.erb (1)
22-23: Manifest colors are hardcoded; dynamic generation is not feasible for PWAs.The review concern is valid: hardcoding dark colors (#0B0B0B) provides poor UX for light mode users, who will see dark PWA splash screens. However, the suggestion to generate the manifest dynamically is not feasible—PWA manifests must be static files served at install time, not per-user.
The app has robust theme support for its UI (user preference stored in DB, Stimulus controller managing light/dark/system modes), but this cannot be applied to the manifest itself.
The current hardcoded dark theme is a deliberate trade-off. Options to address it:
- Accept dark as the default theme for the PWA experience
- Choose a neutral color that works reasonably in both modes
- Document this as a known limitation
No code changes are needed unless the team decides to switch to a different color strategy.
public/site.webmanifest (2)
2-5: LGTM! Standard PWA configuration.The name, short_name, start_url, and scope properties are correctly configured for a Progressive Web App with site-wide scope.
24-25: Dark theme colors look good.The theme and background colors align well with the PR's dark mode focus and provide a cohesive dark appearance for the PWA.
StalkerSea
changed the title
jjmata
added
the
mobile/PWA
Remove apple-touch-icon entry from webmanifest, fix Android icon mislabeling on line 33.
3 participants
Here's another PWA approach, in the screenshot there's an extra space, but that's to consider the white bar on iPhone. Works as intended on the web version as well in the Safari browser.
Sorry for the clutter on the categories!
Works as intended on light mode as well, I just prefer dark mode, so I made changes to the manifest as well to keep everything consistent, not necessary, feel free to test.
Did an extra commit to fix some issues on Windows, they may be freely discarded on UNIX afaik.
Summary by CodeRabbit
New Features
Chores