Bind ModalWideNavigationRail (issue #5 follow-up)#50
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new facade + JNI bridge for Material3 ModalWideNavigationRail (the modal-overlay variant of WideNavigationRail) to complete the remaining actionable follow-up from issue #5, along with a sample usage in the Misc tab.
Changes:
- Introduces a new
ModalWideNavigationRailcomposable container facade with an optionalHeaderslot and documented limitations around dismissal/coroutines. - Adds a
[ComposeBridge]entry for the strippedWideNavigationRailKt::ModalWideNavigationRail-k3FuEkEJVM method plus a corresponding[ComposeDefaults]declaration. - Extends the sample app’s Misc tab to demonstrate mounting/unmounting a modal rail with a “Close” item.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/ComposeNet.Sample/MainActivity.cs | Adds a Misc-tab demo for ModalWideNavigationRail and a visibility gate state. |
| src/ComposeNet.Compose/ModalWideNavigationRail.cs | New facade container that remembers rail state and calls the JNI bridge with appropriate $default masking. |
| src/ComposeNet.Compose/ComposeDefaults.cs | Declares ModalWideNavigationRailDefault bit positions for generator-produced defaults enum. |
| src/ComposeNet.Compose/ComposeBridges.cs | Adds the [ComposeBridge] stub for ModalWideNavigationRail-k3FuEkE targeting the stripped overload. |
PR #33 already shipped CircularProgressIndicator, LinearProgressIndicator, SegmentedButton + both rows, WideNavigationRail, and WideNavigationRailItem for issue #5. This change completes the remaining actionable item from that issue, `ModalWideNavigationRail`. SecureTextField is still deferred on the `TextFieldState` story. - Add `[ComposeBridge] ModalWideNavigationRail` targeting the stripped `ModalWideNavigationRail-k3FuEkE` JVM name (hashed because of the `Dp` `@JvmInline value class` on `collapsedShadowElevation`). - Add `[ComposeDefaults] ModalWideNavigationRailDefault` enum, marking `state` and `content` as `!` (always supplied by the facade). - New `ModalWideNavigationRail` facade (ComposableContainer with optional `Header` slot). Uses the bound (non-stripped) `WideNavigationRailStateKt.RememberWideNavigationRailState` so no extra bridge is needed for the state holder. - Sample: add a "Modal" button to the Misc tab demonstrating the visibility-toggle pattern with an internal "Close" item. The facade documents two limitations callers should know about: 1. The rail always remembers state with `WideNavigationRailValue.Expanded` because the state's `expand`/`collapse`/`snapTo` methods are Kotlin suspend functions (no coroutine story in C# yet). 2. Scrim-tap dismissal hides the rail visually but cannot notify C#; callers gate visibility on their own `MutableState<bool>` and add an internal close button. Once dotnet/java-interop#1440 lands the bridge can be swapped for a direct call to the binding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
668da58 to
3d51fda
Compare
- ModalWideNavigationRail XML doc: use `tab.Value == 0` (was `tab == 0`, which doesn't compile against MutableNumberState<int>). - Sample Misc tab: turn the `Open modal rail` button into a toggle so users can re-mount the rail after a scrim dismiss (the internal Close item is still there for the common case). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Completes the remaining actionable item from #5 that was deferred in #33.
#33 already shipped
CircularProgressIndicator,LinearProgressIndicator,SegmentedButton+ both rows,WideNavigationRail, andWideNavigationRailItem. This change addsModalWideNavigationRail—the modal-overlay variant.
SecureTextFieldis still deferred on theTextFieldStatestory (out of scope).What's added
[ComposeBridge] ModalWideNavigationRailtargets thestripped
androidx/compose/material3/WideNavigationRailKt::ModalWideNavigationRail-k3FuEkEJVM name. The hash comes from the
Dp@JvmInline value classoncollapsedShadowElevation; once [generator] Phase 2: Kotlin @JvmInline value class projection dotnet/java-interop#1440 lands thebridge can be swapped for a direct binding call.
[ComposeDefaults] ModalWideNavigationRailDefaultwith
stateandcontentmarked!(the facade always suppliesthese — they are not enum members and not in
All).ModalWideNavigationRailclass (ComposableContainerwith optional
Headerslot). Uses the bound (non-stripped)WideNavigationRailStateKt.RememberWideNavigationRailStatefor thestate holder, so no extra bridge is needed there.
visibility-toggle pattern with an internal "Close" item.
Documented limitations
WideNavigationRailStatesexpand/collapse/snapTomethods areKotlin
suspendfunctions. We don't have aLaunchedEffect/coroutine story for C# yet, so:
WideNavigationRailValue.Expanded, so the rail opens immediatelywhen mounted.
hideOnCollapse = true) but cannot notify C#. Callers gatevisibility on their own
MutableState<bool>and add an internalclose button — the sample shows the pattern, and the XML doc on
ModalWideNavigationRailcalls this out explicitly.Known risk
The Misc tab still trips compose-net#42 ("LayoutNode insertAt
ArrayIndexOutOfBoundsException") when switching tabs after interacting
with several Misc widgets.
ModalWideNavigationRailmay share the sameunderlying
$changed = 0/ lambda-identity story; tracked separatelyin that issue.
Verified
dotnet test src/ComposeNet.SourceGenerators.Tests— 32/32 pass.dotnet build src/ComposeNet.Compose— 0 errors.dotnet build src/ComposeNet.Sample— 0 errors, 0 warnings.Closes the modal-rail part of #5.