feat(sidebar): let object icons be hidden from a View Options menu - #2036
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
…ray Favorites separator
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.
The sidebar always drew a type icon before every object name, with no way to turn it off. Some people want a plain list of names.
This branch also carries the two compile fixes that were blocking
main(see the last section); #2037 was merged in here, so everything ships together.What it looks like
Where the setting lives
Three entry points, one global preference:
The View menu item is not optional decoration. Apple's guidance on context menus is that "in macOS, an app's menu bar menus list all the app's commands, including those in various context menus", so a command reachable only by right-click is wrong. The menu bar item is the real home; the context menu is the shortcut.
It is a submenu rather than a loose
Show Iconscheckbox because a display toggle is not an action on the clicked table, and the group will grow. Every database client that puts appearance in a context menu uses a submenu for it (DBeaver's Customize view); the ones with flat menus keep appearance out entirely and put it behind a dedicated options button.Scope, and why global
The preference is app-wide, on
GeneralSettingsnext toshowObjectComments, not per connection.DBeaver is the cautionary tale here: its equivalent is scoped to "the currently selected database", and its global default applies only to newly created connections, so an existing user flips the global switch and nothing happens. This is a personal visual preference, not a property of a database. Mail, Safari, VS Code, and macOS's own Sidebar icon size all treat it as global.
Hiding covers tables, views, routines, the database/schema/Recent headers, and Redis namespaces and keys, across the outline tree, both list sidebars, and the Favorites tab. A list that drops table icons but keeps folder icons looks arbitrary, and on Redis the Keys section sits directly under the Tables section in the same list, so leaving it out rendered one list half converted.
Deliberately untouched: the favorite star, the pending truncate and delete badges, the error triangle, and the Favorites tab's own folder and link icons. Those are state and organisation affordances, not database object type icons.
How it renders
.labelStyle(.titleOnly), through a smallsidebarRowIcon(visible:)modifier that sits next to the existingsidebarTint.TitleOnlyLabelStyleremoves the icon from layout rather than hiding it, so names align to the disclosure triangle with no leftover gutter. That gutter is the classic defect in this feature (DBeaver #9735 is exactly it). Indentation per level is independent of the icon, so tree alignment is untouched.The trap this nearly shipped with
The pending truncate/delete badge is an
.overlayon the icon image:Applying
.titleOnlynaively deletes the icon slot and takes the badge with it, so a table staged for deletion would look completely ordinary. The badge now takes over the leading slot when icons are off, so the row keeps its icon slot exactly when it has something to say.TableRowLogic.showsLeadingIconis the pure decision, and it is the thing the new tests pin.Accessibility
Nothing is lost to VoiceOver.
TableRowLogic.accessibilityLabelalready announces the kind (View: active_users) independently of the icon, and a test now pins that it stays true with icons hidden.Icons are not forced back on under Differentiate Without Color. Hiding them is an explicit, reversible, opt-in user choice, and the default is on.
Tests
TableRowLogicTestscovers the leading-icon truth table, including both pending states surviving with icons off.GeneralSettingsObjectIconsTestscovers the default, the round trip, independence fromshowObjectComments, and that settings written before this key existed still decode with icons on.No UI automation: driving a sidebar context menu needs a live connection, so it does not run deterministically in CI.
Also here: the two compile errors that were blocking main
mainat 74a1f54 did not build. Two independent breakages, each from a PR that was green on its own branch and went stale on merge.activeDatabaseNameno longer exists.DatabaseTreeOutlineCoordinator.loadExternalSchemaNamescalledDatabaseManager.shared.activeDatabaseName(for:), which is defined nowhere. #2026 renamed it tobrowseDatabaseName(for:); #2028 branched before the rename and merged after it.browseDatabaseName(for:)is the correct replacement, and its doc comment says so: "Where this connection is being browsed. Use it to seed a new tab and to drive the sidebar."Missing import breaks
#expect.ColumnTypeSQLQuotingTestsfrom #2029 uses#expecton values whose type comes fromTableProPluginKitbut only importedTablePro, so underMemberImportVisibilitythe macro expansion could not seeisTrue/isFalse. Adding the import fixes it.Neither bug reached a release, so neither gets a CHANGELOG entry.