Repository navigation
fix(suiteui): sidebar row toggles keep one column whatever their symbol - #743
Merged
github-actions[bot] merged 1 commit intoOct 14, 2026
Merged
Conversation
Contributor
|
Fast lane: merged into dev (85 lines, 3 files, Fast-Check marker). |
Open
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 15, 2026
…n headers (#938) Refs #763 (dev is red: macOS UI journeys' app audit, run 38669151941 on 0a455df). ## What - `SidebarRow`: each row toggle gets the identifier `suite.sidebar.<item id>.<toggle id>` (`SidebarToggle.identifier(in:)`). #743 hid the toggle's stacked icon from accessibility, so the toggle lost the identifier it used to take from its SF Symbol. - `InspectorSection`: the header button gets `suite.inspector.section.<title slug>` (`InspectorSection.headerIdentifier(_:)`: "Hue/Saturation" becomes `hue-saturation`), and its Reset button gets the same with `.reset`. ## Why I checked each failure against the run's `ui-screens` JSON and compared it with the last green run (38548253551 on ee57a15). The audit message names the wrong element for Flare (factory lesson 59). | App | Audit message | New in the JSON | Fixed by | |---|---|---|---| | design | `checkBox 'Hide Layer'`, `'Lock Layer'`, `'Show Layer'` (23 > 20) | the same three layer-row toggles | sidebar toggle identifier | | flare | `button 'Show Sidebar'` (16 > 15) | `checkBox 'Hide Layer'`, `'Lock Layer'` (Show Sidebar was already in the green run; Hide Inspector left the list when #871 gave it an identifier) | sidebar toggle identifier | | halo | `button 'Layers'` (11 > 10) | `button 'Composition'`, `'Layers'`: the composition inspector's section headers | section header identifier | | library | `menuButton 'Flag'` (17 > 16) | `button 'Metadata'` (section header) and `menuButton 'Flag'` | section header identifier here; Flag in the Library PR | Expected identifier counts after this PR: design 20 or fewer, flare 14, halo 9, library 16 (15 with the Library PR). The headers already in the baselines (Design's Frame, Align, Layer and Fill; Photo's Tool Options, Exposure, Levels and others) drop too, and the `Audit lower` job lowers those baselines. I didn't touch the baseline. Journeys that click a header or toggle by its title still find it, because the query subscript matches any identifying property, the label included (Apple docs: XCUIElementQuery `subscript(_:)` and `matching(identifier:)`). That is the same reason `suite.inspectorToggle` still answers to "Hide Inspector". Library's `offscreen` rise (22 > 16) comes from the window overflow, not from a missing identifier: the split view lays out about 1048 pt in a 900 pt window, so the new metadata inspector sits past the window's right edge. That is #933, on the SuiteUI line. ## Proof - `SidebarTests.sidebarToggleIdentifierNamesItsRowAndKeepsAcrossStates` and `InspectorSectionTests` (new) check the identifiers: stable across a toggle's states, unique across rows and section titles. They are macOS only, like the rest of SuiteUITests. I compiled the slug logic on Linux with `swiftc -swift-version 6`. - No pixels change, so no snapshot PNGs change. The proof is `audit-counts.json` in the next macOS run. - Docs: XCUIElementQuery subscript and `matching(identifier:)` from the apple-docs MCP; `inspector(isPresented:content:)` read for #933. - `scripts/ci` prints `CI: PASS` on the branch rebased on `origin/dev`. Fast-Check: pass Head: 9201ea1 ---
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
SidebarRowdrew each toggle icon from its current SF Symbol only, so the icon was as wide as that symbol. Because the toggles trail aSpacer, a wider symbol (lock.openvslock,eye.slashvseye) pushed every toggle before it sideways. A row with an open lock drew its eye left of the eyes in locked rows.The toggle icon is now a small private
SidebarToggleIcon: aZStackof the toggle's on symbol and off symbol, with only the current one drawn (opacity). The hidden symbol still counts in layout (checked against Apple'shidden()/opacity(_:)docs), so each icon is as wide as the wider of its two symbols in both states. A toggle with the same symbols therefore sits at the same x in every row, and flipping it can't move its neighbours. No hand-sized frame or size token is involved: the symbols set the width. TheLabelkeeps the toggle's title for accessibility, and the icon stack isaccessibilityHidden.I didn't use an alignment guide. In an
HStack, alignment guides only line up views vertically, and each row is its ownHStack, so a shared guide can't make one row's toggles match another row's. The width has to be the same in every state, and stacking both symbols gets that.Proof
SuiteSidebarToggleColumnSnapshots: four rows with eye and lock in every on/off mix (open lock above closed, hidden above visible). The eyes should form one column and the locks another.Tests/SuiteUITests/SidebarRenameGestureTests.swift: newtoggleIconLaysOutBothSymbolsguard. It checks that body draws the icon throughSidebarToggleIcon, never fromtoggle.systemImagealone, and that the icon lays out both symbols with no.frame(. The body call allowlist gainsTextandSidebarToggleIcon. Neither one has a gesture.scripts/ci(Linux):CI: PASS. SuiteUI and the snapshots build only on the macOS job.Snapshot PNGs changed:
suiteui-sidebar-toggles-light.png,suiteui-sidebar-toggles-dark.png(new),suiteui-sidebar-light.png,suiteui-sidebar-dark.png,flare-layers-*,design-layers-*(the eye column in rows with an open lock now lines up).Docs checked with the apple-docs MCP:
Label.init(title:icon:)andView.hidden().Closes #53
Belt-Run: act-1a13827b612-45us
Fast-Check: pass
Head: 2a9dd88