Skip to content

Conversation

ByteBard11
Copy link

@ByteBard11 ByteBard11 commented Aug 4, 2025

Copy of: fix(ui): iOS PWA should take full screen height (maybe-finance#2391, original repo),
with minor updates to handle notification banners & profiler fitting with safe areas.

Changes:

  1. Fixes issue where content did not occupy full viewport height in iOS PWAs, leaving dead space at the top/bottom.
  2. Ensures top status bar and bottom home indicator are theme-aware, restoring immersive dark mode.

Fixes:
Excessive bottom/top padding on iOS 26 (#68)

Tested on iOS 26 16PM

Screenshot 2025-08-04 at 19 02 17 Screenshot 2025-08-04 at 19 03 53

Summary by CodeRabbit

  • Style
    • Improved safe-area handling for devices with notches/home indicators: switched root top spacing to margin, added safe-area padding to dialogs, and removed extra bottom padding from the mobile bottom navigation.
    • Applied a consistent surface background to the page body.
    • Added a styled floating profiler button pattern (visual class only).

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @ByteBard11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing the user experience for iOS Progressive Web Apps by resolving layout issues related to safe area insets. The changes ensure that the application's content correctly utilizes the full screen height, provides a more immersive theme experience by extending backgrounds into safe areas, and properly positions UI components like notifications and the profiler button within the safe display boundaries.

Highlights

  • iOS PWA Full Screen Height: Addressed an issue where content in iOS Progressive Web Apps (PWAs) did not fully occupy the viewport height, leading to dead space at the top and bottom of the screen. This ensures the application utilizes the entire display area.
  • Theme-Aware Safe Areas: Ensured that the top status bar and bottom home indicator areas are theme-aware, restoring an immersive dark mode experience by allowing the application's background to extend seamlessly into these regions.
  • UI Element Positioning for Safe Areas: Adjusted the positioning of key UI elements, including the profiler button and various notification banners (alert, CTA, notice), to correctly account for and respect iOS safe area insets, preventing content from being obscured.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request aims to fix UI layout issues on iOS PWAs by correctly applying safe area insets. The changes correctly add safe area awareness to dialogs, notifications, and the profiler button.

However, I've identified a critical issue with the switch from padding to margin on the <html> element, which will cause unthemed dead space on iOS devices. I've also noted that the bottom navigation for mobile is now missing its necessary bottom padding, which will impact usability. Finally, there's an opportunity to reduce code duplication in how safe area margins are applied to notifications.

My recommendations focus on removing the problematic global margin, restoring the padding on the bottom navigation, and centralizing the notification styling for better maintainability.

@jjmata
Copy link
Collaborator

jjmata commented Aug 5, 2025

@ByteBard11: can you look at the dialog closing bug @steadyfall was running into here? You had quite a bit of review work on the original repo already, so not sure we'll be able to find anybody that can add more value than Zach did there.

I don't see the video anymore, can you re-upload the recording of that bug @steadyfall?

@steadyfall
Copy link
Collaborator

@jjmata I've created a specific issue for it: #84 (comment)

@jjmata jjmata mentioned this pull request Sep 20, 2025
@ByteBard11 ByteBard11 closed this Sep 29, 2025
@ByteBard11 ByteBard11 deleted the pwa_fixes branch September 29, 2025 03:24
@jjmata
Copy link
Collaborator

jjmata commented Sep 29, 2025

This wasn't merged yet, @ByteBard11 ... working on a different PR for it?

@ByteBard11 ByteBard11 restored the pwa_fixes branch September 29, 2025 22:24
@ByteBard11
Copy link
Author

ByteBard11 commented Sep 29, 2025

This wasn't merged yet, @ByteBard11 ... working on a different PR for it?

Apologies @jjmata, I was cleaning up my workspace, this has been restored, and the bottom safe area has been readjusted for issue seen in #149

@alessiocappa alessiocappa mentioned this pull request Sep 29, 2025
@ByteBard11 ByteBard11 reopened this Sep 29, 2025
Copy link

coderabbitai bot commented Sep 29, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Updates safe-area handling and surface styling: adds a .profiler-button rule; switches root safe-area top spacing from padding to margin; adds safe-area top/bottom padding to a dialog; removes bottom safe-area padding from mobile nav; and adds bg-surface to the body classes.

Changes

Cohort / File(s) Summary
Safe-area spacing (root & dialog & nav)
app/assets/tailwind/maybe-design-system.css, app/components/DS/dialog.html.erb, app/views/layouts/application.html.erb
html top spacing changed from padding-top: env(safe-area-inset-top) to margin-top: env(safe-area-inset-top); dialog classes gained pt-[env(safe-area-inset-top)] and pb-[env(safe-area-inset-bottom)]; mobile bottom nav removed pb-[env(safe-area-inset-bottom)].
Styling baseline
app/views/layouts/shared/_htmldoc.html.erb
Adds bg-surface to the body element classes.
Profiler utility
app/assets/tailwind/application.css
Adds .profiler-button with fixed positioning and a safe-area-aware top offset; rule inserted before .combobox in the components layer.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nudge the margins, hop the top,
A button peeks—quick pop and drop.
Dialog cushioned, snug and bright,
The nav breathes easier at night.
Soft surface underfoot—hop, delight. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the core update—fixing safe area insets so that the iOS PWA uses the full screen height—and aligns directly with the PR’s main objective. It avoids unnecessary detail or vague wording and clearly signals the purpose of the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a6cff9 and f9b0bab.

📒 Files selected for processing (5)
  • app/assets/tailwind/application.css (1 hunks)
  • app/assets/tailwind/maybe-design-system.css (1 hunks)
  • app/components/DS/dialog.html.erb (1 hunks)
  • app/views/layouts/application.html.erb (1 hunks)
  • app/views/layouts/shared/_htmldoc.html.erb (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • app/components/DS/dialog.html.erb
  • app/views/layouts/shared/_htmldoc.html.erb
  • app/assets/tailwind/application.css
  • app/views/layouts/application.html.erb
  • 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

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7273878 and bacc069.

📒 Files selected for processing (5)
  • app/assets/tailwind/application.css (1 hunks)
  • app/assets/tailwind/maybe-design-system.css (1 hunks)
  • app/components/DS/dialog.html.erb (1 hunks)
  • app/views/layouts/application.html.erb (1 hunks)
  • app/views/layouts/shared/_htmldoc.html.erb (1 hunks)
🧰 Additional context used
📓 Path-based instructions (25)
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/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/application.css
  • 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/application.css
  • app/assets/tailwind/maybe-design-system.css
  • app/views/layouts/application.html.erb
  • app/views/layouts/shared/_htmldoc.html.erb
  • app/components/DS/dialog.html.erb
**/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
**/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/application.css
  • app/assets/tailwind/maybe-design-system.css
  • app/views/layouts/application.html.erb
  • app/views/layouts/shared/_htmldoc.html.erb
  • app/components/DS/dialog.html.erb
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/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
**/app/assets/tailwind/maybe-design-system.css

📄 CodeRabbit inference engine (CLAUDE.md)

Always reference app/assets/tailwind/maybe-design-system.css for primitives and tokens when using Tailwind CSS.

Files:

  • app/assets/tailwind/maybe-design-system.css
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.yml

Always use the icon helper (icon) for icons; never call lucide_icon directly

Files:

  • app/views/layouts/application.html.erb
  • app/views/layouts/shared/_htmldoc.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.erb
  • app/views/layouts/shared/_htmldoc.html.erb
**/app/**/*.erb

📄 CodeRabbit inference engine (CLAUDE.md)

Always use the icon helper for icons in templates; never use lucide_icon directly.

Files:

  • app/views/layouts/application.html.erb
  • app/views/layouts/shared/_htmldoc.html.erb
  • app/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/application.html.erb
  • app/views/layouts/shared/_htmldoc.html.erb
**/*.{rb,erb,haml,slim}

📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)

**/*.{rb,erb,haml,slim}: Use Current.user for the current user; do not use current_user
Use Current.family for the current family; do not use current_family
Ignore i18n methods; hardcode strings in English for now (do not use I18n.t, t, or similar)

Files:

  • app/views/layouts/application.html.erb
  • app/views/layouts/shared/_htmldoc.html.erb
  • app/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/application.html.erb
  • app/views/layouts/shared/_htmldoc.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.erb
  • app/views/layouts/shared/_htmldoc.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.erb
  • app/views/layouts/shared/_htmldoc.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.erb
  • app/views/layouts/shared/_htmldoc.html.erb
  • app/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/application.html.erb
  • app/views/layouts/shared/_htmldoc.html.erb
  • app/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/application.html.erb
  • app/views/layouts/shared/_htmldoc.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.erb
  • app/views/layouts/shared/_htmldoc.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.erb
  • app/views/layouts/shared/_htmldoc.html.erb
  • app/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/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 (10)
📚 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 : Always reference app/assets/tailwind/maybe-design-system.css for design tokens and primitives; use only defined tokens.

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
📚 Learning: 2025-09-20T08:42:15.420Z
Learnt from: CR
PR: we-promise/sure#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-20T08:42:15.420Z
Learning: Applies to **/app/assets/tailwind/maybe-design-system.css : Always reference `app/assets/tailwind/maybe-design-system.css` for primitives and tokens when using Tailwind CSS.

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
PR: we-promise/sure#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/**,app/javascript/controllers/**} : Style UI using TailwindCSS v4.x with the custom design system defined in app/assets/tailwind/maybe-design-system.css

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
📚 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
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-09-23T22:23:17.441Z
Learning: Applies to app/assets/tailwind/{maybe-design-system.css,application.css} : Do not create new styles in app/assets/tailwind/maybe-design-system.css or app/assets/tailwind/application.css without explicit permission

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
📚 Learning: 2025-09-20T08:42:15.420Z
Learnt from: CR
PR: we-promise/sure#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-20T08:42:15.420Z
Learning: Applies to **/app/assets/tailwind/*.css : NEVER create new styles in design system files without permission.

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
📚 Learning: 2025-09-20T08:42:15.420Z
Learnt from: CR
PR: we-promise/sure#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-20T08:42:15.420Z
Learning: Applies to **/app/assets/tailwind/*.css : 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`).

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
PR: we-promise/sure#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/**,app/javascript/controllers/**} : Always start by referencing app/assets/tailwind/maybe-design-system.css to understand available primitives, functional tokens, and component tokens before styling

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
📚 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/assets/tailwind/maybe-design-system.css
  • app/components/DS/dialog.html.erb
📚 Learning: 2025-09-20T08:42:15.420Z
Learnt from: CR
PR: we-promise/sure#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-20T08:42:15.420Z
Learning: Applies to **/app/views/**/*.erb : Prefer native HTML over JavaScript components for UI elements (e.g., use `<dialog>`, `<details><summary>`).

Applied to files:

  • app/components/DS/dialog.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). (2)
  • GitHub Check: ci / test
  • GitHub Check: ci / test
🔇 Additional comments (4)
app/components/DS/dialog.html.erb (1)

2-2: LGTM: Safe-area padding added to dialog.

The addition of pt-[env(safe-area-inset-top)] and pb-[env(safe-area-inset-bottom)] ensures the dialog content respects iOS safe areas, preventing content from being clipped by the notch or home indicator. This aligns with the per-component safe-area handling strategy.

app/views/layouts/application.html.erb (1)

159-159: Verify mobile nav touch targets remain accessible above home indicator.

The removal of pb-[env(safe-area-inset-bottom)] from the mobile bottom navigation was previously flagged as potentially causing nav items to be obscured by the iOS home indicator. While you've provided screenshot evidence that it works, please confirm that all touch targets remain fully accessible and not overlapped by the home indicator, especially on devices with larger safe-area-insets.

app/assets/tailwind/maybe-design-system.css (1)

270-270: Verify background rendering in top safe-area across browsers.

The change from padding-top to margin-top for env(safe-area-inset-top) was previously flagged as potentially causing visual regressions in dark mode. While you've indicated that adding bg-surface to the body element resolves this, the fix relies on browser behavior that propagates the body's background to the viewport canvas.

Please verify across multiple scenarios:

  • iOS Safari and Chrome
  • Light and dark modes
  • Various safe-area-inset sizes (different device models)

Confirm that no unthemed "dead space" appears in the top safe-area under any combination.

app/views/layouts/shared/_htmldoc.html.erb (1)

16-16: LGTM: Proper use of design system token.

The addition of bg-surface to the body element correctly uses a functional token from the design system. This ensures the body has the appropriate themed background color.

As per coding guidelines.

@alessiocappa
Copy link

alessiocappa commented Oct 2, 2025

@ByteBard11 I'm doing some tests, but I still have some weird behaviours in PWA mode on iOS 26.0.1.
If I scroll starting from the bottom nav bar, I can scroll the outer container and the nav bar is moving as well:

Screenshot 2025-10-02 (18 18 34)

It seems like the outer element (maybe html or body?) has an height that is higher than the device screen, but I'm not sure if it's related to that.

Do you experience the same behaviour on your device? I can try to investigate a bit to see if I can fix it.

@ByteBard11
Copy link
Author

@ByteBard11 I'm doing some tests, but I still have some weird behaviours in PWA mode on iOS 26.0.1. If I scroll starting from the bottom nav bar, I can scroll the outer container and the nav bar is moving as well:
Do you experience the same behaviour on your device? I can try to investigate a bit to see if I can fix it.

Hi alessiocappa, I don't have this issue. I can't scroll the page with the bottom bar if the app is added to your home page. I can scroll down in safari like you describe, but that's not in PWA mode.

@alessiocappa
Copy link

@ByteBard11 I'm doing some tests, but I still have some weird behaviours in PWA mode on iOS 26.0.1. If I scroll starting from the bottom nav bar, I can scroll the outer container and the nav bar is moving as well:
Do you experience the same behaviour on your device? I can try to investigate a bit to see if I can fix it.

Hi alessiocappa, I don't have this issue. I can't scroll the page with the bottom bar if the app is added to your home page. I can scroll down in safari like you describe, but that's not in PWA mode.

Thanks for the feedback, in my case I can reproduce the issue on both Safari and in PWA mode.

I tried to look into it a bit, and I believe the issue is related to how the overflow is managed; since the main section is the only part that can be scrolled (you can see it also on a desktop, the scroll bar appears next to the chart when you scroll the main content), this create some conflicts in case of mobile view.

I believe the best would be to apply a fixed position to the sidebar when in desktop mode, and let the whole html/body scroll. The sidebar is hidden on mobile, so it should not be a problem.

I partially managed to test it but I haven’t finished yet, I can try to work on it in the next few days and share the results to see how it works on different devices.

…#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.
@ByteBard11
Copy link
Author

Thanks for the feedback, in my case I can reproduce the issue on both Safari and in PWA mode.

I tried to look into it a bit, and I believe the issue is related to how the overflow is managed; since the main section is the only part that can be scrolled (you can see it also on a desktop, the scroll bar appears next to the chart when you scroll the main content), this create some conflicts in case of mobile view.

I believe the best would be to apply a fixed position to the sidebar when in desktop mode, and let the whole html/body scroll. The sidebar is hidden on mobile, so it should not be a problem.

I partially managed to test it but I haven’t finished yet, I can try to work on it in the next few days and share the results to see how it works on different devices.

Thanks @alessiocappa, I can't seem to replicate your issue at all - not even in Safari either anymore. Scrolling on the navbar doesn't have any effect for me. Are you able to send me a screen recording when you get a chance? I've synced/rebased this branch if you would like to repull to help with your fixes if required.

@alessiocappa
Copy link

Thanks for the feedback, in my case I can reproduce the issue on both Safari and in PWA mode.
I tried to look into it a bit, and I believe the issue is related to how the overflow is managed; since the main section is the only part that can be scrolled (you can see it also on a desktop, the scroll bar appears next to the chart when you scroll the main content), this create some conflicts in case of mobile view.
I believe the best would be to apply a fixed position to the sidebar when in desktop mode, and let the whole html/body scroll. The sidebar is hidden on mobile, so it should not be a problem.
I partially managed to test it but I haven’t finished yet, I can try to work on it in the next few days and share the results to see how it works on different devices.

Thanks @alessiocappa, I can't seem to replicate your issue at all - not even in Safari either anymore. Scrolling on the navbar doesn't have any effect for me. Are you able to send me a screen recording when you get a chance? I've synced/rebased this branch if you would like to repull to help with your fixes if required.

Here's the recording, it's from the PWA mode:

ScreenRecording_10-03-2025.20-15-07_1.MP4

I'll try to move forward with my testing and see if I can fix it. Is it fine if I push my changes to your branch? Or is it better if I do it in a different PR?

@alessiocappa
Copy link

I think I found a way to solve the issue.
Basically, I added an external div which act as a container for the whole content; it's fixed on mobile, with safe area insets paddings. This avoids the content from scrolling in mobile, while keeping the same behaviour on desktop.

Additionally, I also removed the scrollbar on mobile, to make it feeling more like a native app, and I've adjusted some paddings that were different between pages and way too big on mobile.

I don't know how to update the PR, so I put the changes here since they are quite easy:

diff --git a/app/assets/tailwind/maybe-design-system.css b/app/assets/tailwind/maybe-design-system.css
index e8fb9488..3419c77f 100644
--- a/app/assets/tailwind/maybe-design-system.css
+++ b/app/assets/tailwind/maybe-design-system.css
@@ -266,11 +266,6 @@
     --shadow-xl: 0px 20px 24px -4px --alpha(var(--color-white) / 8%);
   }
 
-  html {
-    margin-top: env(safe-area-inset-top);
-    padding-bottom: env(safe-area-inset-bottom);
-  }
-
   button {
     @apply cursor-pointer focus-visible:outline-gray-900;
   }
@@ -444,4 +439,11 @@
       fill: var(--color-white);
     }
   }
+
+  @media (hover: none) {
+    ::-webkit-scrollbar {
+      display: none;
+    }
+  }
+
 }
\ No newline at end of file
diff --git a/app/views/budgets/show.html.erb b/app/views/budgets/show.html.erb
index de47b27c..11c417d2 100644
--- a/app/views/budgets/show.html.erb
+++ b/app/views/budgets/show.html.erb
@@ -1,4 +1,4 @@
-<div class="pb-12">
+<div class="lg:pb-12">
   <%= render "budgets/budget_header",
             budget: @budget,
             previous_budget: @previous_budget,
diff --git a/app/views/layouts/shared/_htmldoc.html.erb b/app/views/layouts/shared/_htmldoc.html.erb
index d1d7dd9e..b6d79709 100644
--- a/app/views/layouts/shared/_htmldoc.html.erb
+++ b/app/views/layouts/shared/_htmldoc.html.erb
@@ -7,38 +7,40 @@
   data-theme="<%= theme %>"
   data-controller="theme"
   data-theme-user-preference-value="<%= Current.user&.theme || "system" %>"
-  class="h-full text-primary overflow-hidden lg:overflow-auto font-sans <%= @os %>">
+  class="h-screen text-primary overflow-hidden lg:overflow-auto font-sans <%= @os %>">
   <head>
     <%= render "layouts/shared/head" %>
     <%= yield :head %>
   </head>
 
   <body class="h-full bg-surface overflow-hidden lg:overflow-auto antialiased">
-    <% if Rails.env.development? %>
-      <button hidden data-controller="hotkey" data-hotkey="t t /" data-action="theme#toggle"></button>
-    <% end %>
+    <div class="fixed inset-0 lg:relative h-full pt-[env(safe-area-inset-top)] pb-[env(safe-area-inset-bottom)]">
+      <% if Rails.env.development? %>
+        <button hidden data-controller="hotkey" data-hotkey="t t /" data-action="theme#toggle"></button>
+      <% end %>
 
-    <div class="fixed z-50 top-6 md:top-4 left-1/2 -translate-x-1/2 w-full md:w-80 px-4 md:px-0 mx-auto md:mx-0 md:right-auto mt-safe">
-      <div id="notification-tray" class="space-y-1 w-full">
-        <%= render_flash_notifications %>
+      <div class="fixed z-50 top-6 md:top-4 left-1/2 -translate-x-1/2 w-full md:w-80 px-4 md:px-0 mx-auto md:mx-0 md:right-auto mt-safe">
+        <div id="notification-tray" class="space-y-1 w-full">
+          <%= render_flash_notifications %>
 
-        <div id="cta"></div>
+          <div id="cta"></div>
+        </div>
       </div>
-    </div>
 
-    <% if Current.family %>
-      <%= turbo_stream_from Current.family %>
-    <% end %>
+      <% if Current.family %>
+        <%= turbo_stream_from Current.family %>
+      <% end %>
 
-    <%= turbo_frame_tag "modal" %>
-    <%= turbo_frame_tag "drawer" %>
+      <%= turbo_frame_tag "modal" %>
+      <%= turbo_frame_tag "drawer" %>
 
-    <%# Custom overrides for browser's confirm API  %>
-    <%= render "layouts/shared/confirm_dialog" %>
+      <%# Custom overrides for browser's confirm API  %>
+      <%= render "layouts/shared/confirm_dialog" %>
 
-    <%= render "impersonation_sessions/super_admin_bar" if Current.true_user&.super_admin? && show_super_admin_bar? %>
-    <%= render "impersonation_sessions/approval_bar" if Current.true_user&.impersonated_support_sessions&.initiated&.any? %>
+      <%= render "impersonation_sessions/super_admin_bar" if Current.true_user&.super_admin? && show_super_admin_bar? %>
+      <%= render "impersonation_sessions/approval_bar" if Current.true_user&.impersonated_support_sessions&.initiated&.any? %>
 
-    <%= yield %>
+      <%= yield %>
+    </div>
   </body>
 </html>
diff --git a/app/views/pages/dashboard.html.erb b/app/views/pages/dashboard.html.erb
index 4e06b270..3820f063 100644
--- a/app/views/pages/dashboard.html.erb
+++ b/app/views/pages/dashboard.html.erb
@@ -23,7 +23,7 @@
   </div>
 <% end %>
 
-<div class="w-full space-y-6 pb-24">
+<div class="w-full space-y-6 lg:pb-12">
   <% if Current.family.accounts.any? %>
     <%= turbo_frame_tag "cashflow_sankey_section" do %>
       <section class="bg-container py-4 rounded-xl shadow-border-xs mb-6">
diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb
index 8fa5860c..9f3406ce 100644
--- a/app/views/transactions/index.html.erb
+++ b/app/views/transactions/index.html.erb
@@ -1,4 +1,4 @@
-<div class="space-y-4 pb-20 flex flex-col">
+<div class="space-y-6 lg:pb-12 flex flex-col">
   <header class="flex justify-between items-center text-primary font-medium">
     <h1 class="text-xl">Transactions</h1>
     <div class="flex items-center gap-5">

Let me know if you can test it and if you have any feedback/comment to share

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants