Skip to content

Commit f8edee7

Browse files
CopilotShreckYe
andauthored
Fix *Select state sync on JS DOM via value-based selection (#81)
* Initial plan * Fix *Select state sync on JS DOM by using value-based selection - Pass value=textFieldArgs.valueComposeUi to MdFilledSelect/MdOutlinedSelect so Material Web uses value property setter (select(value)) to reliably set displayed selection from Compose state - Remove selected attribute approach in SelectOption JS impl since selection is now controlled via the parent select's value attribute - Add valueJsDom param to SelectOptionWithMaterialIcons - Update demo SelectMenuContent to pass valueJsDom=it?.name?:'' so each option has a proper value matching the select element's value Co-authored-by: ShreckYe <27768951+ShreckYe@users.noreply.github.com> * Add valueJsDom param to FilledSelect/OutlinedSelect; use it instead of valueComposeUi The valueComposeUi field in SelectTextFieldArgs is the Compose UI text-field display string, while Material Web's value attribute is an HTML form key used for programmatic option selection — semantically different. - Select.kt: add valueJsDom: String to FilledSelect and OutlinedSelect - Select.js.kt: use valueJsDom instead of textFieldArgs.valueComposeUi - Select.composeUi.kt: accept valueJsDom param (suppressed; unused on this platform) - Demo: pass value (selection?.name ?: "") as valueJsDom Co-authored-by: ShreckYe <27768951+ShreckYe@users.noreply.github.com> * Clarify valueJsDom vs valueComposeUi distinction in comments Co-authored-by: ShreckYe <27768951+ShreckYe@users.noreply.github.com> * Review all the changes in this PR and fix unindented changes - Manually test and make `valueJsDom` not-null across all functions. - Adjust the parameter order in `SelectOption`. * Move inline comments to KDoc; move selection comment above commented-out selected line Co-authored-by: ShreckYe <27768951+ShreckYe@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ShreckYe <27768951+ShreckYe@users.noreply.github.com> Co-authored-by: Yongshun Ye <ShreckYe@gmail.com>
1 parent c8eaaf4 commit f8edee7

File tree

4 files changed

+48
-32
lines changed
  • demo/src/commonMain/kotlin/com/huanshankeji/compose/material/demo
  • material3/src
    • commonMain/kotlin/com/huanshankeji/compose/material3/ext
    • composeUiMain/kotlin/com/huanshankeji/compose/material3/ext
    • jsMain/kotlin/com/huanshankeji/compose/material3/ext

4 files changed

+48
-32
lines changed

demo/src/commonMain/kotlin/com/huanshankeji/compose/material/demo/Material3.kt

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,17 +202,17 @@ fun Material3(/*modifier: Modifier = Modifier*/
202202
}
203203

204204
@Composable
205-
fun SelectMenuContent(selection: Selection?, setSelection: (Selection?) -> Unit, close: () -> Unit) =
205+
fun SelectMenuContent(setSelection: (Selection?) -> Unit, close: () -> Unit) =
206206
(listOf(null) + Selection.entries).forEach {
207207
SelectOptionWithMaterialIcons(
208208
{ modifier -> it?.let { Text(it.name, modifier) } },
209209
{
210210
setSelection(it)
211211
close()
212212
},
213-
it == selection,
213+
it?.name ?: "",
214214
leadingIcon = Icons.Filled.Add,
215-
trailingIcon = Icons.Filled.Remove
215+
trailingIcon = Icons.Filled.Remove,
216216
)
217217
}
218218

@@ -245,14 +245,19 @@ fun Material3(/*modifier: Modifier = Modifier*/
245245
)
246246
}
247247

248+
// `valueJsDom` doesn't necessarily need to be the same as `value`. See its KDoc for details.
249+
val valueJsDom = selection?.name ?: ""
250+
// an alternative using `ordinal`
251+
//val valueJsDom = selection?.run { ordinal.toString() } ?: ""
248252
run {
249253
val (expanded, setExpanded) = remember { mutableStateOf(false) }
250254
val close = { setExpanded(false) }
251255
FilledSelect(
252256
expanded, setExpanded,
257+
valueJsDom,
253258
textFieldArgs = SelectTextFieldArgs(value, label = label),
254259
menuArgs = SelectMenuArgs(expanded, close, close) {
255-
SelectMenuContent(selection, setSelection, close)
260+
SelectMenuContent(setSelection, close)
256261
}
257262
)
258263
}
@@ -261,9 +266,10 @@ fun Material3(/*modifier: Modifier = Modifier*/
261266
val close = { setExpanded(false) }
262267
OutlinedSelect(
263268
expanded, setExpanded,
269+
valueJsDom,
264270
textFieldArgs = SelectTextFieldArgs(value, label = label),
265271
menuArgs = SelectMenuArgs(expanded, close, close) {
266-
SelectMenuContent(selection, setSelection, close)
272+
SelectMenuContent(setSelection, close)
267273
}
268274
)
269275
}

material3/src/commonMain/kotlin/com/huanshankeji/compose/material3/ext/Select.kt

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,13 @@ class SelectMenuArgs(
3535
* - On Compose UI: Uses [ExposedDropdownMenuBoxWithTextField] with a filled text field
3636
* - On JS: Uses native Material Web `MdFilledSelect` component
3737
*
38-
* @param value the currently selected value
39-
* @param onValueChange called when the selected value changes
4038
* @param modifier the [Modifier] to be applied to this select
41-
* @param enabled controls the enabled state of this select
42-
* @param label optional label for this select
43-
* @param options the options to display in the dropdown, typically [SelectOption] calls
39+
* @param valueJsDom the HTML form key for the `md-filled-select` element, used internally to
40+
* identify which option is selected. By convention, it should be a code-like identifier string
41+
* (no spaces, e.g. `"OPTION_A"` or `"option_a"`), NOT a natural-language display string. This is
42+
* semantically different from [SelectTextFieldArgs.valueComposeUi], which is the text shown in
43+
* the text field (may contain spaces, e.g. `"Option A"`). Also, it can't be null — the
44+
* `md-filled-select` component doesn't react to a `null` value change on JS DOM.
4445
*
4546
* @see <a href="https://m3.material.io/components/menus/overview">Material Design select menus</a>
4647
*
@@ -51,15 +52,8 @@ class SelectMenuArgs(
5152
expect fun FilledSelect(
5253
expandedComposeUi: Boolean,
5354
onExpandedChangeComposeUi: (Boolean) -> Unit,
54-
/*
55-
These 2 params are temporarily not supported because their API style differ too much from Compose.
56-
Also note that the `value` here comes from `MdFilledSelect` and serves as the key,
57-
which differs from `value` in `ExposedDropdownMenuBoxTextFieldArgs` that serves as the text field value.
58-
*/
59-
/*
6055
valueJsDom: String,
61-
onValueChangeJsDom: (String) -> Unit,
62-
*/
56+
//onValueChangeJsDom: (String) -> Unit,
6357
modifier: Modifier = Modifier,
6458
textFieldArgs: SelectTextFieldArgs,
6559
//scrollState: ScrollState = rememberScrollState(),
@@ -68,12 +62,16 @@ expect fun FilledSelect(
6862

6963
/**
7064
* The outlined variant of [FilledSelect].
65+
*
66+
* @param valueJsDom see the `valueJsDom` parameter in [FilledSelect] for the semantics.
7167
*/
7268
@ExperimentalApi
7369
@Composable
7470
expect fun OutlinedSelect(
7571
expandedComposeUi: Boolean,
7672
onExpandedChangeComposeUi: (Boolean) -> Unit,
73+
valueJsDom: String,
74+
//onValueChangeJsDom: (String) -> Unit,
7775
modifier: Modifier = Modifier,
7876
textFieldArgs: SelectTextFieldArgs,
7977
//scrollState: ScrollState = rememberScrollState(),
@@ -87,37 +85,43 @@ expect fun OutlinedSelect(
8785
*
8886
* @see DropdownMenuItem
8987
* @param text corresponds to the `headline` slot on JS DOM in Material Web.
90-
* @param valueJsDom the `value` attribute for an HTML form.
88+
* @param valueJsDom the HTML form key `value` attribute. By convention this should be a code-like
89+
* identifier (no spaces), distinct from any natural-language display string.
90+
* Also see the `valueJsDom` parameter in [FilledSelect] and [OutlinedSelect],
91+
* which is used to identify the currently selected option.
9192
*/
9293
@Composable
9394
expect fun SelectOption(
9495
text: @Composable (Modifier) -> Unit,
9596
onClick: () -> Unit,
96-
selectedJsDom: Boolean,
97+
// An alternative parameter to identify the currently selected option on JS DOM. This doesn't work properly when syncing states on JS DOM, however.
98+
//selectedJsDom: Boolean,
99+
valueJsDom: String,
97100
modifier: Modifier = Modifier,
98101
leadingIcon: @Composable ((Modifier) -> Unit)? = null,
99102
trailingIcon: @Composable ((Modifier) -> Unit)? = null,
100103
enabled: Boolean = true,
101-
valueJsDom: String? = null,
102-
//keepOpenJsDom: Boolean = false
104+
//keepOpenJsDom: Boolean = false,
103105
)
104106

105107
@Composable
106108
fun SelectOptionWithMaterialIcons(
107109
text: @Composable (Modifier) -> Unit,
108110
onClick: () -> Unit,
109-
selectedJsDom: Boolean,
111+
//selectedJsDom: Boolean,
112+
valueJsDom: String,
110113
modifier: Modifier = Modifier,
111114
leadingIcon: Icon? = null,
112115
trailingIcon: Icon? = null,
113-
enabled: Boolean = true
116+
enabled: Boolean = true,
114117
) =
115118
SelectOption(
116119
text,
117120
onClick,
118-
selectedJsDom,
121+
//selectedJsDom,
122+
valueJsDom,
119123
modifier,
120124
leadingIcon.toNullableContentWithModifier(),
121125
trailingIcon.toNullableContentWithModifier(),
122-
enabled
126+
enabled,
123127
)

material3/src/composeUiMain/kotlin/com/huanshankeji/compose/material3/ext/Select.composeUi.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ fun SelectMenuArgs.toComposeUiExposedDropdownMenuArgs() =
2525
actual fun FilledSelect(
2626
expandedComposeUi: Boolean,
2727
onExpandedChangeComposeUi: (Boolean) -> Unit,
28+
@Suppress("UNUSED_PARAMETER") valueJsDom: String,
2829
modifier: Modifier,
2930
textFieldArgs: SelectTextFieldArgs,
3031
menuArgs: SelectMenuArgs
@@ -41,6 +42,7 @@ actual fun FilledSelect(
4142
actual fun OutlinedSelect(
4243
expandedComposeUi: Boolean,
4344
onExpandedChangeComposeUi: (Boolean) -> Unit,
45+
@Suppress("UNUSED_PARAMETER") valueJsDom: String,
4446
modifier: Modifier,
4547
textFieldArgs: SelectTextFieldArgs,
4648
menuArgs: SelectMenuArgs
@@ -57,11 +59,11 @@ actual fun OutlinedSelect(
5759
actual fun SelectOption(
5860
text: @Composable ((Modifier) -> Unit),
5961
onClick: () -> Unit,
60-
selectedJsDom: Boolean,
62+
//selectedJsDom: Boolean,
63+
valueJsDom: String,
6164
modifier: Modifier,
6265
leadingIcon: @Composable ((Modifier) -> Unit)?,
6366
trailingIcon: @Composable ((Modifier) -> Unit)?,
6467
enabled: Boolean,
65-
valueJsDom: String?
6668
) =
6769
CommonDropdownMenuItem(text, onClick, modifier, leadingIcon, trailingIcon, enabled)

material3/src/jsMain/kotlin/com/huanshankeji/compose/material3/ext/Select.js.kt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ private fun mdSelectAttrs(
3434
actual fun FilledSelect(
3535
expandedComposeUi: Boolean,
3636
onExpandedChangeComposeUi: (Boolean) -> Unit,
37+
valueJsDom: String,
3738
modifier: Modifier,
3839
textFieldArgs: SelectTextFieldArgs,
3940
menuArgs: SelectMenuArgs
@@ -43,6 +44,7 @@ actual fun FilledSelect(
4344
supportingText = textFieldArgs.supportingText,
4445
error = textFieldArgs.isError.isTrueOrNull(),
4546
disabled = textFieldArgs.enabled.isFalseOrNull(),
47+
value = valueJsDom,
4648
clampMenuWidth = menuArgs.matchAnchorWidth,
4749
attrs = mdSelectAttrs(modifier, menuArgs)
4850
) {
@@ -53,6 +55,7 @@ actual fun FilledSelect(
5355
actual fun OutlinedSelect(
5456
expandedComposeUi: Boolean,
5557
onExpandedChangeComposeUi: (Boolean) -> Unit,
58+
valueJsDom: String,
5659
modifier: Modifier,
5760
textFieldArgs: SelectTextFieldArgs,
5861
menuArgs: SelectMenuArgs
@@ -62,6 +65,7 @@ actual fun OutlinedSelect(
6265
supportingText = textFieldArgs.supportingText,
6366
error = textFieldArgs.isError.isTrueOrNull(),
6467
disabled = textFieldArgs.enabled.isFalseOrNull(),
68+
value = valueJsDom,
6569
attrs = mdSelectAttrs(modifier, menuArgs)
6670
) {
6771
menuArgs.content()
@@ -75,18 +79,18 @@ actual fun OutlinedSelect(
7579
actual fun SelectOption(
7680
text: @Composable ((Modifier) -> Unit),
7781
onClick: () -> Unit,
78-
selectedJsDom: Boolean,
82+
//selectedJsDom: Boolean,
83+
valueJsDom: String,
7984
modifier: Modifier,
8085
leadingIcon: @Composable ((Modifier) -> Unit)?,
8186
trailingIcon: @Composable ((Modifier) -> Unit)?,
8287
enabled: Boolean,
83-
valueJsDom: String?
8488
) =
8589
// copied and adapted from `DropdownMenuItem`
8690
MdSelectOption(
8791
enabled.isFalseOrNull(),
88-
// `selected` has to be passed to update the selection from Compose data. This doesn't work properly now when syncing states, however.
89-
selected = selectedJsDom.isTrueOrNull(),
92+
// Selection is controlled via the `value` attribute on the parent select element, not via `selected` here.
93+
//selected = selectedJsDom.isTrueOrNull(),
9094
value = valueJsDom,
9195
attrs = modifier.toAttrs {
9296
onClick { onClick() }

0 commit comments

Comments
 (0)