Skip to content

fix: adapt full-width modal controls to vertical bars - #229

Merged
rdlabo merged 6 commits into
mainfrom
fix/modal-vertical-bars-insets
Sep 27, 2026
Merged

rdlabo merged 6 commits into
mainfrom
fix/modal-vertical-bars-insets

Conversation

@rdlabo

@rdlabo rdlabo commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Full-width Ionic modals now project eligible toolbar controls, including Close, into the Vertical Control Area. Centered dialogs retain their toolbar layout without inheriting the page's rail inset. Full-width sheets follow their visible bounds as their breakpoint changes, matching the SwiftUI behavior observed on the iPhone Duo simulator.

The existing Web and native projection paths share modal eligibility and foreground selection. Only the topmost modal contributes controls; dismissal restores the previous surface and preserves Ionic's canDismiss behavior. Web controls remain inside the modal for focus handling. Existing page toolbar offsets are unchanged.

Validation:

  • Ionic 9: 106 E2E tests passed; 25 layout/back-button tests rerun after the supervisor review fix.
  • Ionic 8: 36 E2E tests passed in an isolated dependency environment.
  • Swift: 24 tests passed; lifecycle/transition unit tests: 14 passed.
  • Package build and formatting checks passed.
  • Supervisor review: approved after fixing the page toolbar offset regression.

Devin Review

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Playwright test results

passed  363 passed

Details

stats  363 tests across 20 suites
duration  4 minutes, 1 second
commit  05a1c7e
info  This detailed result covers Ionic 9 only. Ionic 8 runs against the same screenshots in a separate matrix job; check the workflow run for both results. To update the screenshots, comment with /update-screenshots.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Devin Review

Comment thread src/styles/vertical-bars.scss Outdated
Comment on lines 61 to 63
ion-app.ios-theme-vertical-bars ion-content:is(.ios, .md):not(.ios-theme-disabled, .ios26-disabled):not(:where(ion-menu *, ion-popover *)) {
--ion-safe-area-left: 0px;
--ion-safe-area-right: 0px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 中央表示のモーダルで指定済みのセーフエリア余白が消える

中央表示のモーダルに --ion-safe-area-right: 12px を指定しても、内部のコンテンツとツールバーには 0px が適用されます。モーダル固有の余白が失われ、内容やボタンが意図より端に寄ります。

Learn more

この変更で、レール用の ion-content と ion-toolbar のセレクターが、中央表示の ion-modal の子要素にも一致するようになりました。両セレクターは --ion-safe-area-left と --ion-safe-area-right を 0px に上書きします。モーダル本体の resolved inset を 0px にしても、モーダルが独自に設定した Ionic のセーフエリア値まで消す必要はありません。同じ問題は ツールバーのセレクター にもあります。

Example: アプリが中央表示のモーダルに --ion-safe-area-right: 12px を設定すると、従来は内部の ion-content に 12px が継承されました。変更後は 0px となり、指定した右余白が消えます。

Recommended fix: ion-content と ion-toolbar のセレクターを通常ページまたは .ios-theme-vertical-bars-modal 内に限定し、中央表示のモーダルには従来の Ionic のセーフエリア処理を残してください。モーダル内で独自の --ion-safe-area-* を設定するケースも検証してください。

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +80 to +85
if (
top &&
(pointerActive ||
Array.from(wanted).some((element) => element.getAnimations().some((animation) => animation.playState === 'running')))
)
schedule();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 実行中のアニメーションによる継続的な寸法監視

表示中のモーダルのコンテンツに実行中のアニメーションがある間、寸法が変わらなくても毎フレーム全モーダルを走査します。長時間のカスタムアニメーションを使うアプリへの影響を確認してください。

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

github-actions Bot added a commit that referenced this pull request Sep 26, 2026
github-actions Bot added a commit that referenced this pull request Sep 26, 2026
@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

📊 Ionic 9 Playwright Test Report

View the detailed Ionic 9 report: https://rdlabo-dev.github.io/ionic-theme-ios27/pr-229/

Ionic 8 runs against the same screenshots in a separate matrix job. View both results in the workflow run.

@rdlabo

rdlabo commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

/update-screenshots

@github-actions

Copy link
Copy Markdown
Contributor

ℹ️ No screenshot changes detected.

The current screenshots are already up to date.

github-actions Bot added a commit that referenced this pull request Sep 27, 2026
github-actions Bot added a commit that referenced this pull request Sep 27, 2026
github-actions Bot added a commit that referenced this pull request Sep 27, 2026
github-actions Bot added a commit that referenced this pull request Sep 27, 2026
@rdlabo
rdlabo merged commit 4838a85 into main Sep 27, 2026
11 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

npm beta published

CI passed for the merge commit 4838a8540978. Install the immutable version with:

npm install @rdlabo/ionic-theme-ios27@1.2.0-beta.pr229.sha4838a8540978

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.

1 participant