Re: Safe Area Inset Fixes - PWA should take full screen height #237
Conversation
…#2391 maybe-finance#2391 With updated _notice tab safety area. Further fix: adjust padding to margin for safe area insets across notify compents Fix profiler location Good DRY fix from gemini Restore bottom safe area. Should be small adjust to allow for device return bar, etc.
WalkthroughRemoved global html safe-area padding and overflow-hidden; applied safe-area padding to dialog, body, notification container, and certain mobile elements; added a fixed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/assets/tailwind/application.css(1 hunks)app/assets/tailwind/maybe-design-system.css(0 hunks)app/components/DS/dialog.html.erb(1 hunks)app/views/layouts/application.html.erb(3 hunks)app/views/layouts/shared/_htmldoc.html.erb(1 hunks)
💤 Files with no reviewable changes (1)
- app/assets/tailwind/maybe-design-system.css
🧰 Additional context used
📓 Path-based instructions (23)
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/_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/_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/_htmldoc.html.erbapp/assets/tailwind/application.cssapp/views/layouts/application.html.erbapp/components/DS/dialog.html.erb
**/app/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Always use the
iconhelper for icons in templates; never uselucide_icondirectly.
Files:
app/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erbapp/components/DS/dialog.html.erb
**/app/views/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/views/**/*.erb: Keep domain logic OUT of view templates; logic belongs in component files, not templates.
Use partials only for primarily static, simple HTML with minimal logic, and only when not likely to be reused.
Prefer native HTML over JavaScript components for UI elements (e.g., use<dialog>,<details><summary>).
Leverage Turbo frames for page sections over client-side JavaScript solutions.
Use query params (not localStorage/sessions) for client state management.
Perform server-side formatting for currencies, numbers, and dates in templates.
Files:
app/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/views/layouts/shared/_htmldoc.html.erbapp/assets/tailwind/application.cssapp/views/layouts/application.html.erbapp/components/DS/dialog.html.erb
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
app/views/layouts/shared/_htmldoc.html.erbapp/views/layouts/application.html.erbapp/components/DS/dialog.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/_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/_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/_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/_htmldoc.html.erbapp/views/layouts/application.html.erbapp/components/DS/dialog.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/_htmldoc.html.erbapp/views/layouts/application.html.erbapp/components/DS/dialog.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/_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/_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/_htmldoc.html.erbapp/views/layouts/application.html.erbapp/components/DS/dialog.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/_htmldoc.html.erb
app/assets/**/*.{css,scss,sass,pcss,png,jpg,jpeg,gif,svg,webp,woff,woff2,ttf,otf}
📄 CodeRabbit inference engine (AGENTS.md)
Store styles and static assets (Tailwind, images, fonts) under app/assets/
Files:
app/assets/tailwind/application.css
app/assets/**/*.{css,scss,sass,pcss}
📄 CodeRabbit inference engine (AGENTS.md)
Let Biome format CSS/SCSS styles (npm run lint/format)
Files:
app/assets/tailwind/application.css
**/app/assets/tailwind/*.css
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/assets/tailwind/*.css: NEVER create new styles in design system files without permission.
Use functional tokens (e.g.,text-primary,bg-container) as defined in the design system; do not use hardcoded color classes (e.g.,text-white,bg-white).
Files:
app/assets/tailwind/application.css
app/assets/tailwind/{maybe-design-system.css,application.css}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Do not create new styles in app/assets/tailwind/maybe-design-system.css or app/assets/tailwind/application.css without explicit permission
Files:
app/assets/tailwind/application.css
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
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.erb
**/app/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/components/**/*: Component logic should live in component files; keep domain logic out of view templates.
Prefer components over partials when available.
Files:
app/components/DS/dialog.html.erb
🧠 Learnings (1)
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
PR: we-promise/sure#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.erb
🔇 Additional comments (5)
app/views/layouts/shared/_htmldoc.html.erb (2)
10-10: LGTM! Background theming improvement.The addition of
bg-surfaceto the html tag ensures consistent background theming across the document, which aligns with the dark mode safe area theming mentioned in the PR objectives.
16-16: LGTM! Scroll containment fix.Moving
overflow-hiddento the body element (from the html element) is part of the scroll fix strategy mentioned in the PR objectives to prevent scrolling from escaping the PWA viewport.app/components/DS/dialog.html.erb (1)
2-2: LGTM! Proper safe-area handling for mobile PWA.The addition of safe-area inset padding ensures dialog content doesn't overlap with device notches, home indicators, or status bars on iOS and Android devices. This is essential for PWA mobile support.
app/views/layouts/application.html.erb (2)
35-35: LGTM! Prevents unintended touch interactions.Adding
touch-noneto the mobile top navigation prevents users from accidentally initiating scrolls when interacting with the navigation bar, which is appropriate for fixed UI chrome.
114-114: LGTM! Improved mobile scroll behavior.The addition of
overscroll-containprevents scroll chaining (bounce effects propagating to parent containers), and[-webkit-overflow-scrolling:touch]enables smooth momentum scrolling on iOS devices. Both are best practices for PWA mobile experiences.
Removed duplicate padding from mobile navigation. Signed-off-by: ByteBard <[email protected]>
|
Thanks, @ByteBard11 ... will give it a quick look/test today and push. You wouldn't have any experience with Hotwire Native, would you. :-) I just had Codex push this draft PR, completely untested still. |
|
@ByteBard11, thanks for the update. I still have some weird behaviour on my device: 
It seems like the top padding isn’t being taken into account. All the header is overlapping with the Dynamic Island, while the bottom is still too far from the edge. I’m not sure if this is only me, but I’m testing it on an iPhone 15 with the latest iOS. |
Thanks @alessiocappa, I've restored the CSS changes, they didn't have any affect on my device, but seem to affect your device. Cheers |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/assets/tailwind/maybe-design-system.css(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
app/assets/**/*.{css,scss,sass,pcss,png,jpg,jpeg,gif,svg,webp,woff,woff2,ttf,otf}
📄 CodeRabbit inference engine (AGENTS.md)
Store styles and static assets (Tailwind, images, fonts) under app/assets/
Files:
app/assets/tailwind/maybe-design-system.css
app/assets/**/*.{css,scss,sass,pcss}
📄 CodeRabbit inference engine (AGENTS.md)
Let Biome format CSS/SCSS styles (npm run lint/format)
Files:
app/assets/tailwind/maybe-design-system.css
app/assets/tailwind/maybe-design-system.css
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/assets/tailwind/maybe-design-system.css: Always reference app/assets/tailwind/maybe-design-system.css for design tokens and primitives; use only defined tokens.
NEVER add new styles to the design system CSS file without permission.
Files:
app/assets/tailwind/maybe-design-system.css
**/*.{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/assets/tailwind/maybe-design-system.css
**/app/assets/tailwind/maybe-design-system.css
📄 CodeRabbit inference engine (CLAUDE.md)
Always reference
app/assets/tailwind/maybe-design-system.cssfor primitives and tokens when using Tailwind CSS.
Files:
app/assets/tailwind/maybe-design-system.css
**/app/assets/tailwind/*.css
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/assets/tailwind/*.css: NEVER create new styles in design system files without permission.
Use functional tokens (e.g.,text-primary,bg-container) as defined in the design system; do not use hardcoded color classes (e.g.,text-white,bg-white).
Files:
app/assets/tailwind/maybe-design-system.css
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/assets/tailwind/maybe-design-system.css
app/assets/tailwind/{maybe-design-system.css,application.css}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Do not create new styles in app/assets/tailwind/maybe-design-system.css or app/assets/tailwind/application.css without explicit permission
Files:
app/assets/tailwind/maybe-design-system.css
🧠 Learnings (1)
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
PR: we-promise/sure#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to app/assets/tailwind/maybe-design-system.css : NEVER add new styles to the design system CSS file without permission.
Applied to files:
app/assets/tailwind/maybe-design-system.css
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
This reverts commit 63ef953.
I’ll be on business trip this week, I can test it next week if it’s ok. However, I’ve seen the AI comment and I kinda agree: it’s strange to use margins instead of paddings for this specific case. |
jjmata
added
the
mobile/PWA
The issue is that it seems to be applying the safe area twice, once in the CSS for the entire html and then another in application.html.erb. I've reverted and just removed the html css and then corrected divs in application.html.erb so that the header bar doesn't go into the status bar. Seems like the html element still doesn't extend the whole screen in PWA mode unless you correct the main div with min-h-[100dvh]. |
I’ve tested it now and confirmed that it works. The elements are displayed properly, and there are no more overflow issues. However, I also experience the same height bug. I noticed the same issue while trying to fix it myself. I guess there are some JavaScript scripts running to dynamically modify the height, but that’s just an assumption. EDIT: Sorry, I didn’t notice that settings section has still some issues: 
The top menu is overlapping with the notch/Dynamic Island area. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/views/layouts/settings.html.erb (1)
9-9: Good consistency with overscroll improvements.The
overscroll-containand-webkit-overflow-scrolling:touchadditions match the main application layout, providing consistent PWA scrolling behavior across layouts.Note: The
pb-20provides fixed spacing, but on devices with large bottom safe areas (like iPhones with home indicators), consider whetherpb-[calc(5rem+env(safe-area-inset-bottom,0))]would provide better spacing to prevent content from being obscured.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/views/layouts/application.html.erb(3 hunks)app/views/layouts/settings.html.erb(2 hunks)app/views/layouts/shared/_htmldoc.html.erb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/views/layouts/shared/_htmldoc.html.erb
🧰 Additional context used
📓 Path-based instructions (12)
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/application.html.erbapp/views/layouts/settings.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/application.html.erbapp/views/layouts/settings.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/application.html.erbapp/views/layouts/settings.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/application.html.erbapp/views/layouts/settings.html.erb
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
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/application.html.erbapp/views/layouts/settings.html.erb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/views/layouts/application.html.erbapp/views/layouts/settings.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/application.html.erbapp/views/layouts/settings.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/application.html.erbapp/views/layouts/settings.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/application.html.erbapp/views/layouts/settings.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/layouts/application.html.erbapp/views/layouts/settings.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/application.html.erbapp/views/layouts/settings.html.erb
🧠 Learnings (7)
📚 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/views/layouts/application.html.erbapp/views/layouts/settings.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/**/*.erb : Be mindful of performance in global layouts (e.g., avoid loading large data payloads)
Applied to files:
app/views/layouts/application.html.erbapp/views/layouts/settings.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/**/*.html.erb : Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Applied to files:
app/views/layouts/application.html.erbapp/views/layouts/settings.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/application.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 partials when content is primarily static, simple, context-specific, and without variants or complex configuration
Applied to files:
app/views/layouts/application.html.erbapp/views/layouts/settings.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/application.html.erbapp/views/layouts/settings.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/shared/**/_*.html.erb : Place shared partials in app/views/shared/
Applied to files:
app/views/layouts/application.html.erb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (2)
app/views/layouts/application.html.erb (1)
114-114: Excellent PWA scrolling improvements.The
overscroll-containprevents scroll-chaining and rubber-band effects from escaping the viewport, while-webkit-overflow-scrolling:touchenables smooth momentum scrolling on iOS. Both are essential for a native-like PWA experience.app/views/layouts/settings.html.erb (1)
7-7: Layout container refactor looks good.Moving
overflow-y-autofrom the outer main to the inner content wrapper (line 9) provides better control over the scrollable region while maintaining proper flex layout.
Fixed mobile scrolling on setting page
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/views/layouts/settings.html.erb(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
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/settings.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/settings.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/settings.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/settings.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/settings.html.erb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/views/layouts/settings.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/settings.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/settings.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/settings.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/layouts/settings.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/settings.html.erb
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:17:53.155Z
Learning: PRs should include clear description, linked issues, screenshots for UI changes, and migration notes; ensure CI, tests, RuboCop, and Biome are clean
📚 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/**/*.erb : Be mindful of performance in global layouts (e.g., avoid loading large data payloads)
Applied to files:
app/views/layouts/settings.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/**/*.html.erb : Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Applied to files:
app/views/layouts/settings.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/**/*.html.erb : Prefer native HTML elements (e.g., <dialog>, <details><summary>) over JS-based components
Applied to files:
app/views/layouts/settings.html.erb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (2)
app/views/layouts/settings.html.erb (2)
7-7: LGTM: Proper flex container setup.The addition of
grow flex h-fullcorrectly establishes the main content area as a flexible container that takes full available height.
26-26: LGTM: Consistent footer spacing.The addition of
mb-4provides symmetrical vertical spacing for the footer container.
|
@alessiocappa thanks for your support. I've also adjust the bottom padding to be a bit smaller than the full safe area. I've tested across a couple simulators in Xcode and look fine now. |
No concerns! Let me test with this and the other PWA commits that came in today, and push after that sanity check. |
3 participants
Moved from #69 due to clutter.
Further fixed to remove scroll issues reported in previous.
Fixes:
There is a bug in the theme controller somewhere (untouched), which has an issue with the top status bar:
If iOS set to dark mode & Sure is set to light mode, then the status bar will still be incorrectly dark.
If iOS is set to light mode & Sure is set to dark mode, then the status bar will be correctly dark
I still think this should be pushed regardless as an improvement for mobile users.
Before/After:
Credits: @alessiocappa & original PR: (maybe-finance#2391, original repo)
Summary by CodeRabbit