fix(ui): iOS PWA should take full screen height #2391

Closed

KenTandrian wants to merge 16 commits from bugfix/ui-pwa-ios into main

pull from: bugfix/ui-pwa-ios

merge into: gavin:main

gavin:main

gavin:zachgoll/maybe-872-budgets-no-longer-allow-user-to-go-back-to-previous-months

gavin:zachgoll/entry-drawer-form-behavior

gavin:zachgoll/maybe-902-add-anchor-fields-to-valuation

gavin:zachgoll/security-price-fix

gavin:zachgoll/partial-syncs

gavin:zachgoll/ai-skeleton

gavin:1948-some-graph-periods-are-not-reporting-enough-points-daily-vs-monthly-data

gavin:1888-bug-debts-not-summed-correctly-for-account-balances

gavin:1615-pre-load-exchange-rates-and-security-prices-in-the-background

gavin:missing-securities-error

gavin:zachgoll/budgeting

gavin:863-improvement-allow-user-to-configure-date-and-amount-format-during-transaction-imports

gavin:1087-feature-support-multi-currency-syncing-for-investment-account-portfolios

gavin:1077-bug-error-when-creating-transactions-for-investment-account

Conversation 13 Commits 16 Files Changed 4 +4 -4

KenTandrian commented 2025-06-18 12:55:48 +08:00 (Migrated from github.com)

Copy Link

This PR fixes #2342, which comprises these issues:

• The page content was not taking full height of the iOS screen in PWA, resulting in dead space at the bottom.
• The top status bar and the bottom home indicator were not theme-aware, breaking the immersive dark mode experience.

Changes

• Set env(safe-area-inset-top) as top margin instead of top padding for html.
• Remove safe-area-inset-bottom padding from the bottom navigation component since html has it.
• Ensured body adapts background color by dynamically setting it to bg-surface.
• Add top and bottom padding based on safe area height to dialog component.

After

This PR fixes #2342, which comprises these issues: * The page content was not taking full height of the iOS screen in PWA, resulting in dead space at the bottom. * The top status bar and the bottom home indicator were not theme-aware, breaking the immersive dark mode experience. ## Changes * Set `env(safe-area-inset-top)` as top margin instead of top padding for `html`. * Remove `safe-area-inset-bottom` padding from the bottom navigation component since `html` has it. * Ensured `body` adapts background color by dynamically setting it to `bg-surface`. * Add top and bottom padding based on safe area height to dialog component. ## After <img alt="Dark mode" src="https://github.com/user-attachments/assets/e01ce8e4-1e01-402f-bd92-e2b0fb6ade27" width="300" /> <img alt="Light mode" src="https://github.com/user-attachments/assets/fff77644-4e22-4b1a-a387-4b49ebd10098" width="300" />

👍 3

zachgoll (Migrated from github.com) reviewed 2025-06-18 12:55:48 +08:00

zachgoll (Migrated from github.com) requested changes 2025-06-23 22:29:20 +08:00

zachgoll (Migrated from github.com) left a comment

Copy Link

Thanks for the fix here, but this seems to cause several visual regressions to the desktop version of the app. Below are just a few examples, but I'm sure there are several more:

We need to be super careful editing these files as they apply to many views across the app.

Thanks for the fix here, but this seems to cause several visual regressions to the desktop version of the app. Below are just a few examples, but I'm sure there are several more:   We need to be super careful editing these files as they apply to many views across the app.

KenTandrian commented 2025-06-24 01:22:18 +08:00 (Migrated from github.com)

Copy Link

@zachgoll Thanks for the feedback. My bad for removing h-full from body, resulting in the page not taking full height.

I have reverted some of the changes and only focus on 4 things:

1. Set env(safe-area-inset-top) as top margin instead of top padding for html.
2. Remove safe-area-inset-bottom padding from the bottom navigation component since html has it.
3. Ensured body adapts background color by dynamically setting it to bg-surface.
4. Add top and bottom padding based on safe area height to dialog component.

Edit: added dialog padding.

@zachgoll Thanks for the feedback. My bad for removing `h-full` from `body`, resulting in the page not taking full height. I have reverted some of the changes and only focus on 4 things: 1. Set `env(safe-area-inset-top)` as top margin instead of top padding for `html`. 2. Remove `safe-area-inset-bottom` padding from the bottom navigation component since `html` has it. 3. Ensured `body` adapts background color by dynamically setting it to `bg-surface`. 4. Add top and bottom padding based on safe area height to dialog component. Edit: added dialog padding.

albertorizzi commented 2025-06-24 01:26:05 +08:00 (Migrated from github.com)

Copy Link

@KenTandrian can you fix also modals? I can't close.

@KenTandrian can you fix also modals? I can't close. 

KenTandrian commented 2025-06-24 01:56:42 +08:00 (Migrated from github.com)

Copy Link

@albertorizzi done!

@albertorizzi done! <img alt="Dark mode" src="https://github.com/user-attachments/assets/2786b982-0656-46d3-938d-83253881c987" width="300" /> <img alt="Light mode" src="https://github.com/user-attachments/assets/ddc56c12-f2b8-499e-ac5c-59a3877ab139" width="300" />

🚀 1

albertorizzi commented 2025-06-24 02:14:37 +08:00 (Migrated from github.com)

Copy Link

Wow! Thanks a lot!😍

Wow! Thanks a lot!😍

albertorizzi commented 2025-06-24 02:16:49 +08:00 (Migrated from github.com)

Copy Link

Your PR https://github.com/maybe-finance/maybe/pull/2410 fix also the Current Market Price "Unknown" in these screens?

Your PR https://github.com/maybe-finance/maybe/pull/2410 fix also the Current Market Price "Unknown" in these screens?

KenTandrian commented 2025-06-24 13:10:31 +08:00 (Migrated from github.com)

Copy Link

Your PR #2410 fix also the Current Market Price "Unknown" in these screens?

Yes, I think that should be covered since @holding.security.current_price is retrieved from the same find_or_fetch_price method.

> Your PR #2410 fix also the Current Market Price "Unknown" in these screens? Yes, I think that should be covered since [`@holding.security.current_price`](https://github.com/maybe-finance/maybe/blob/c0617f74cdadd9f96c8701158a35c5274d3f450a/app/views/holdings/show.html.erb#L24) is retrieved from the same [`find_or_fetch_price`](https://github.com/maybe-finance/maybe/blob/c0617f74cdadd9f96c8701158a35c5274d3f450a/app/models/security.rb#L15) method.

❤️ 1

albertorizzi commented 2025-06-24 17:58:47 +08:00 (Migrated from github.com)

Copy Link

Is it possible to improve the mobile view of the Holdings tab?

Is it possible to improve the mobile view of the Holdings tab? 

KenTandrian commented 2025-06-24 21:23:43 +08:00 (Migrated from github.com)

Copy Link

Is it possible to improve the mobile view of the Holdings tab?

I don't think it's related to PWA screen height? It should be a separate issue.

> Is it possible to improve the mobile view of the Holdings tab? I don't think it's related to PWA screen height? It should be a separate issue.

👍 3

KenTandrian commented 2025-06-27 23:21:02 +08:00 (Migrated from github.com)

Copy Link

Hi @zachgoll, a gentle follow up on this PR.

I've pushed fixes to address the desktop regressions you pointed out and have also resolved the modal issue reported by @albertorizzi. I believe all requested changes are complete.

Please let me know if there's anything else needed when you have a moment. Thanks!

Hi @zachgoll, a gentle follow up on this PR. I've pushed fixes to address the desktop regressions you pointed out and have also resolved the modal issue reported by @albertorizzi. I believe all requested changes are complete. Please let me know if there's anything else needed when you have a moment. Thanks!

👍 2

zachgoll (Migrated from github.com) reviewed 2025-06-30 21:55:28 +08:00

zachgoll (Migrated from github.com) left a comment

Copy Link

Hey @KenTandrian, sorry this is a bit of a tough one to review just given this will affect every page in our app.

I haven't had a chance to dig deeply into this yet, but my gut tells me we really need to keep that existing "global padding" intact on our html element, then apply specific padding/margin adjustments to things like our Dialog to make them look good on mobile. Changing the global property to margin introduces a lot of weird UI states on "full-bleed" sections as I had screenshotted earlier, so I don't think we should be using that here.

So I think in summary what we should probably do is:

• Revert global CSS changes (keep existing padding safe areas)
• Apply specific safe area padding to fixed elements like dialogs, mobile navs, etc.

Hey @KenTandrian, sorry this is a bit of a tough one to review just given this will affect every page in our app. I haven't had a chance to dig deeply into this yet, but my gut tells me we really need to keep that existing "global padding" intact on our `html` element, then apply specific padding/margin adjustments to things like our Dialog to make them look good on mobile. Changing the global property to margin introduces a lot of weird UI states on "full-bleed" sections as I had screenshotted earlier, so I don't think we should be using that here. So I think in summary what we should probably do is: - Revert global CSS changes (keep existing padding safe areas) - Apply specific safe area padding to fixed elements like dialogs, mobile navs, etc.

KenTandrian commented 2025-07-02 02:26:29 +08:00 (Migrated from github.com)

Copy Link

Hey @zachgoll, it's tricky to implement this without touching the html element at all.

To illustrate why, I have reverted all my changes and added borders to highlight the default sizing:

1. Green border: html
2. Blue border: body
3. Purple border: nav (mobile bottom nav)

 
As you can see from the screenshot, the html element isn't taking up the full height of the screen by default. This then prevents the body and nav elements from reaching the bottom as well. Because of this, I don't think changes isolated to just the dialog or mobile nav would be enough to resolve the layout issue.

Do you have any ideas on how we could address the html element's height?

Hey @zachgoll, it's tricky to implement this without touching the `html` element at all. To illustrate why, I have reverted all my changes and added borders to highlight the default sizing: 1. Green border: `html` 2. Blue border: `body` 3. Purple border: `nav` (mobile bottom nav) <img alt="Dark mode" src="https://github.com/user-attachments/assets/db7195a7-c64e-4185-a341-936fa00fdd68" width="300" /> &nbsp; As you can see from the screenshot, the `html` element isn't taking up the full height of the screen by default. This then prevents the `body` and `nav` elements from reaching the bottom as well. Because of this, I don't think changes isolated to just the dialog or mobile nav would be enough to resolve the layout issue. Do you have any ideas on how we could address the `html` element's height?

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

Reviewers

No Reviewers

zachgoll

Labels

Clear labels

📆 Planned

Not ready to be worked on yet, but planned for future

💻 Self Hosted only

Issues pertaining to self-hosted versions of Maybe

⚙️ Config

ℹ️ Needs info

Further information is requested

📱 Mobile

Bugs exclusively happening on mobile devices

1️⃣ High Priority

High priority issues will be worked on next

🚨 Urgent

Issues that are urgent for app functionality

3️⃣ Low priority

Contributions accepted, but Maybe team will not be working on this in the near term

2️⃣ Medium Priority

Community contributions accepted, Maybe team only works on if there are no high priority items open

⚡ Performance

Performance issues to address

🎨 UI / UX

Issues related to UI / UX interactions and design.

💎 Bounty

💰 Rewarded

📊 Data issue

Issues where the app functionally works, but the data is missing or inaccurate.

🙋 Bounty claim

$1.5K

Community

This is a great issue for community members to work on

dependencies

Pull requests that update a dependency file

Design

Hosted only

Hosted app only

Required self hoster data migration

Self hosters must run data migrations to resolve this issue

ruby

Pull requests that update Ruby code

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

gavin

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gavin/maybe#2391