Skip to content

fix: align toolbar and modal button geometry with system controls - #263

Merged
rdlabo merged 21 commits into
mainfrom
investigate/toolbar-projection-geometry
Oct 4, 2026
Merged

rdlabo merged 21 commits into
mainfrom
investigate/toolbar-projection-geometry

Conversation

@rdlabo

@rdlabo rdlabo commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Toolbar actions have inconsistent outer insets and spacing when individual buttons and ion-buttons share a toolbar side. Multiple clear-button groups are more compact than the standard native toolbar layout, and card/sheet modal headers place controls too close to the top edge.

Align toolbar typography, grouped action sizing, outer insets and adjacent-action spacing with the measured native baseline. Multiple clear-button groups use 16px between their inner controls, matching standard SwiftUI toolbar groups. Handle Ionic’s visual slot order (start + secondary, primary + end) and exclude hidden back buttons and sources moved to native Vertical Bars from adjacent-action spacing. Preserve optional back-button text, small/large sizes, opted-out controls and source CSS geometry used by Native UI Shell.

Card/sheet headers place 44px controls 16px below the content edge. At the existing large-viewport breakpoint, non-sheet modal headers retain a 10px inset; sheet headers retain the 16px inset. Ionic’s own modal presentation remains in control, including landscape layouts.

Production changes are confined to two SCSS files, and updated screenshot baselines are included. No Swift changes or additional test files are included. Existing screenshot coverage remains the visual guardrail.

Validation: root lint and diff checks; 12 existing native projection/browser tests; 11-scene CLI XCTest comparison of main and this branch with Native UI Shell enabled on iPhone 17 Pro; iPhone/iPad layout measurements; focused checks for single/multiple actions, RTL and excluded groups. Temporary diagnostic code remains outside the repository. The native toolbar comparison includes standard-app screenshots and a same-icon SwiftUI baseline; small subpoint differences remain.

Empty groups and groups containing only hidden children no longer contribute toolbar spacing. The toolbar demo at /main/index/toolbar includes 14 spacing examples covering both sides, hidden and empty groups, standalone buttons, and paired slots for screenshot coverage.


Devin Review

@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.

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 4 potential issues.

Devin Review

Comment thread src/styles/components/ion-button.scss Outdated
Comment thread src/styles/components/ion-button.scss Outdated
Comment thread src/styles/components/ion-button.scss Outdated
padding-top: 0;
// The system's modal safe area places the 44px control 16px below the
// content edge, with or without a sheet drag indicator.
padding-top: 6px;

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.

🔍 モーダルの上余白は端末種別ではなく画面寸法で切り替わる

幅768px以上かつ高さ600px以上なら上余白は0px、それ以外は6pxです。横向きや分割表示で想定する配置を確認してください。

Devin Review


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

@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 1 new potential issue.

Devin Review

Comment thread src/styles/components/ion-toolbar.scss Outdated
@rdlabo

rdlabo commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

/update-screenshots

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

✅ Screenshots have been updated successfully!

The new screenshots have been committed to this PR.

@rdlabo

rdlabo commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

/update-screenshots

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

✅ Screenshots have been updated successfully!

The new screenshots have been committed to this PR.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Playwright test results

passed  389 passed
flaky  1 flaky

Details

stats  390 tests across 21 suites
duration  5 minutes, 53 seconds
commit  dc83f89
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.

Flaky tests

chromium › vertical-bars-back-button.spec.ts › Web search follows vertical tabs before native projection and restores its source

github-actions Bot added a commit that referenced this pull request Oct 3, 2026
@github-actions

github-actions Bot commented Oct 3, 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-263/

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

@rdlabo

rdlabo commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

/update-screenshots

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

✅ Screenshots have been updated successfully!

The new screenshots have been committed to this PR.

github-actions Bot added a commit that referenced this pull request Oct 3, 2026
github-actions Bot added a commit that referenced this pull request Oct 3, 2026
@rdlabo

rdlabo commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

/update-screenshots

github-actions Bot added a commit that referenced this pull request Oct 3, 2026
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

✅ Screenshots have been updated successfully!

The new screenshots have been committed to this PR.

@rdlabo

rdlabo commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Native toolbar geometry comparison

Measured on the same iPhone 17 Pro simulator (iOS 27.0). The multiple-clear-button groups now follow native toolbar spacing.

Files’ Back/Forward controls and Calendar’s Search/Add controls both have 16pt between their button frames.

Files and Calendar standard toolbar layouts

For a controlled comparison, the standard SwiftUI ToolbarItemGroup uses the same Ionicons images at the same intrinsic size, without custom button padding, spacing, or frames.

Measurement main (852ce7bc) This PR (639a6a09) SwiftUI, same icons
Group width 88.66pt 99.25pt 100pt
Visible icon-to-icon spacing, 3× screenshot 79px 108px 109px

main, this PR, and SwiftUI using identical icons at the same scale

The icons themselves are unchanged in size. The group is wider because its internal layout now follows the native baseline. A small subpoint difference remains; this is not a claim of exact pixel equality for arbitrary icons.

github-actions Bot added a commit that referenced this pull request Oct 3, 2026
github-actions Bot added a commit that referenced this pull request Oct 3, 2026
github-actions Bot added a commit that referenced this pull request Oct 3, 2026
@rdlabo

rdlabo commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

/update-screenshots

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

✅ Screenshots have been updated successfully!

The new screenshots have been committed to this PR.

github-actions Bot added a commit that referenced this pull request Oct 3, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor

再レビュー(head: d81c6cf1)

結論: ブロッカーはありません。 これまでの指摘はすべて解消されています。

main と PR それぞれでビルドした CSS を、headless Chromium と WebKit(Playwright、390px 幅)で表示し、寸法を測って確認しました。

ケース(slot="start") main PR
非表示の menu だけを含む group の次の group x=40 x=16
空の group をはさんだ 2 group の間隔 — 12px
非表示の back button/.ios-theme-native-ui-shell-prehidden の子だけを含む group の次 — x=16
空の opt-out group の後に 2 group — 16 / 12px(間隔が維持される)
visible な opt-out group と通常 group — 12px + 12px(standalone の余白が維持される)
start/secondary、primary/end、end の 2 group — 外側 16px/間隔 12px
  • :nth-child(1 of …) と、:has() を入れ子にしたセレクタが WebKit でも有効なことを確認しました。生成された 14 ルールはすべて CSSOM に読み込まれ、レイアウトも Chromium と一致しています。
  • ion-buttons に新しく付けた display: none が Native UI Shell の要素の読み取りと干渉しないか、コードで確認しました。
    • withoutPrehide() は対象要素と祖先のクラスを外します。unprojected() は data-native-ui-shell の marker を外します。そのため、子を読み取るあいだは親の group が表示状態に戻ります。
    • group 自体を読み取るのは、group 全体が持ち主になる経路だけです。この経路では子に prehidden が付かないので、問題になる経路は見つかりませんでした。ただし実機では確認していません。
  • .back-button-has-icon-only の TODO(v2.0.0)、PR 説明の更新、デモの 14 ケースとスクリーンショットも確認しました。

任意

  • Nit: 要素を含まずテキストだけの <ion-buttons>テキスト</ion-buttons> も、:not(:has(> …)) に当てはまるため display: none になります。Ionic の想定する使い方ではないので、気にならなければこのままで構いません。
  • この PR とは別件(main と同じ挙動): 非表示の ion-menu-button と一緒に置いた ion-button には、:has(ion-menu-button.menu-button-hidden) の除外によってガラスの背景が付きません。デモの「hidden child and visible child」で、Save だけ背景がないのはこのためです。

Written by Devin

github-actions Bot added a commit that referenced this pull request Oct 3, 2026
github-actions Bot added a commit that referenced this pull request Oct 3, 2026
github-actions Bot added a commit that referenced this pull request Oct 3, 2026
@rdlabo

rdlabo commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

/update-screenshots

@github-actions

github-actions Bot commented Oct 3, 2026

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 Oct 3, 2026
github-actions Bot added a commit that referenced this pull request Oct 3, 2026
devin-ai-integration Bot and others added 2 commits October 4, 2026 00:33
Co-Authored-By: rdlabo <sakakibara@rdlabo.jp>
Co-Authored-By: rdlabo <sakakibara@rdlabo.jp>
github-actions Bot added a commit that referenced this pull request Oct 4, 2026
@rdlabo
rdlabo merged commit 154af91 into main Oct 4, 2026
11 checks passed
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

npm beta published

CI passed for the merge commit 154af91ad6d7. Install the immutable version with:

npm install @rdlabo/ionic-theme-ios27@1.2.0-beta.pr263.sha154af91ad6d7

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