Show context-menu separators only where the renderer draws them - #565
Merged
Conversation
Spellcheck used to inject an empty ContextMenuItem sentinel and then replace LocalContextMenuRepresentation with a hand-rolled Compose menu just to paint a hairline for it. That swapped the app's chrome for hardcoded colors driven by isSystemInDarkTheme(), lost keyboard navigation and accessibility, and only kicked in when the clicked word was misspelled — so the menu changed appearance between two right clicks in the same field. Renderers now publish their own divider through LocalContextMenuDivider: NativeContextMenuProvider provides NucleusContextMenuDivider (drawn by NSMenu on macOS and by the Fluent / Adwaita / Breeze flyouts elsewhere), ProvideJewelSpellcheckMenu provides Jewel's ContextMenuDivider, and the default is null. Compose's own representation cannot draw a divider, so the spellcheck sections are emitted without one instead of inventing chrome. Apps that want dividers without Jewel already have a supported path: nativeContextMenu = true. Also normalize interpreted entries in NativeContextMenuRepresentation, trimming leading and trailing separators and collapsing runs, since menu items come from several independent contributors. The jewel-demo chrome test asserted representation identity, which only held while SpellChecker had no warm session; it now probes that the ambient chrome is what ultimately draws.
The Adwaita flyout painted popover_shade_color, which is 25% black in
the dark variant, so menu separators came out darker than the menu
surface. libadwaita draws them from $border_color instead:
separator { background: $border_color; }
$border_color: color-mix(in srgb, currentColor var(--border-opacity), transparent)
--border-opacity: 15%
currentColor inside a menu is popover_fg_color, so the rule is 15% of
the text color: white on a dark menu, and RGB(0 0 6 / 80%) premultiplied
down to 12% on a light one (the light variant was at 7%).
popover_shade_color stays what libadwaita uses it for, scroll
undershoots.
Breeze and Fluent already lightened their dark surfaces; the new test
pins that invariant for the three themes plus the 15% rule for Adwaita.
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.
Summary
LocalContextMenuRepresentation. The hand-rolled Compose menu that existed only to paint a hairline for an empty-label sentinel is gone (187 lines), along withSpellcheckContextMenuSeparatorandLocalSpellcheckMenuSeparator. That popup used hardcoded colors driven byisSystemInDarkTheme(), dropped keyboard navigation and a11y, and only engaged when the clicked word was misspelled — so the menu looked different between two right clicks in the same field.LocalContextMenuDivider(nullable, defaultnull) is provided byNativeContextMenuProvider(NucleusContextMenuDivider, drawn byNSMenuon macOS and by the Fluent / Adwaita / Breeze flyouts elsewhere) and byProvideJewelSpellcheckMenu(Jewel'sContextMenuDivider). With Compose's own representation there is no divider to draw, so none is emitted. Apps wanting dividers outside Jewel already have a supported path:nativeContextMenu = true.spellcheckMenuSections/spellcheckContextMenuItems/NucleusSpellcheckInstaller.menuItemstake a nullable separator;ContextMenuEntry.ktno longer imports thespellcheckpackage.NativeContextMenuRepresentationnormalizes interpreted entries — trims leading/trailing separators, collapses runs, recurses into submenus — since menu items come from several independent contributors (field Cut/Copy/Paste, app extras, spellcheck).popover_shade_color(25 % black), so the rule came out darker than the menu surface. libadwaita usesseparator { background: $border_color }with$border_color: color-mix(in srgb, currentColor var(--border-opacity), transparent)and--border-opacity: 15%— i.e. 15 % of the text color: white on a dark menu,RGB(0 0 6 / 80%)premultiplied to 12 % on a light one (was 7 %). Breeze and Fluent were already correct.apiDumprefreshed. The removed symbols were never published (latest release is 2.4.4), so nothing is deprecated.Test plan
./gradlew :nucleus-application:check :decorated-window-jewel:check— detekt, ktlint, apiCheck, tests./gradlew :examples:jewel-demo:test :examples:jewel-demo:ktlintCheckContextMenuDividerCapabilityTest— no divider published by default,NucleusContextMenuDividerunder the OS-looking provider, none when it is disabledContextMenuFlyoutSeparatorTest— Adwaita's 15 %-of-currentColorrule, and dark separators lighten / light separators darken the surface across Adwaita, Breeze and FluentSpellcheckInstallerTest— no separator and no stand-in row when the renderer cannot draw one, for both placements./gradlew :examples:tao-demo:compileKotlin :examples:nucleus-demo:compileKotlinNote:
top placement keeps Jewel chromefailed onnucleus-2.5before this branch — itsassertSameon the representation only held whileSpellCheckerhad no warm session. It now probes that the ambient chrome is what ultimately draws, for both placements.