From 40a37dfd929e9aa888c10080a51020364ca193fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 5 Oct 2026 13:31:15 -0400 Subject: [PATCH] Choices: review r2 fixes --- cascader.go | 7 +- choice_frame_test.go | 114 +++++++++++++++++++++++++++++++ combobox.go | 5 +- docs/components/034-slider.md | 3 +- examples/gallery/pages_choice.go | 2 +- multi_select.go | 4 +- select.go | 29 ++++++-- slider.go | 109 ++++++++++++++++++----------- slider_grid_test.go | 74 ++++++++++++++++++++ tree_select.go | 7 +- 10 files changed, 300 insertions(+), 54 deletions(-) create mode 100644 choice_frame_test.go create mode 100644 slider_grid_test.go diff --git a/cascader.go b/cascader.go index de1e38c..186eb47 100644 --- a/cascader.go +++ b/cascader.go @@ -79,7 +79,7 @@ func Cascader(c *ui.Context, path *[]string, opts CascaderOptions) ChoiceResult if opts.Disabled { st.open = false } - if tr.Clicked() || (!st.open && tr.Shortcut(0, ui.KeyDown)) { + if tr.Clicked() || (!st.open && tr.Shortcut(0, ui.KeyDown) && !tr.IsDisabled()) { st.open = !st.open if st.open { st.active = slices.Clone(*path) @@ -170,10 +170,11 @@ func cascaderColumn(c *ui.Context, st *cascaderState, path *[]string, opts Casca selectNote(c, msg(c, "cascader.empty")) default: ui.Scroll(c).MaxHeight(260).Padding(4).Children(func() { + entry := slices.IndexFunc(lv.Nodes, func(n CascaderNode) bool { return !n.Disabled }) for i, n := range lv.Nodes { on := depth < len(st.active) && st.active[depth] == n.Value deeper = deeper || on && !n.Leaf - cascaderRow(c, st, path, opts, prefix, n, on, i == 0) + cascaderRow(c, st, path, opts, prefix, n, on, i == entry) } }) } @@ -208,7 +209,7 @@ func cascaderRow(c *ui.Context, st *cascaderState, path *[]string, opts Cascader st.active, *path, st.open = full, full, false case clicked: open(false) - case b.Shortcut(0, ui.KeyRight) && !n.Leaf: + case b.Shortcut(0, ui.KeyRight) && !n.Leaf && !b.IsDisabled(): open(true) case b.Shortcut(0, ui.KeyLeft) && depth > 0: st.focus, st.focusSet = slices.Clone(prefix), true diff --git a/choice_frame_test.go b/choice_frame_test.go new file mode 100644 index 0000000..c3c5f07 --- /dev/null +++ b/choice_frame_test.go @@ -0,0 +1,114 @@ +package mujica + +import ( + "slices" + "testing" + + "github.com/egoist/mygo/ui" +) + +// Keys delivered in the frame a control or option turns disabled must change nothing. + +func TestMultiSelectBackspaceAsItDisables(t *testing.T) { + values, disabled := []int{1, 2}, false + tt := choiceBetween(func(c *ui.Context) { + MultiSelect(c, &values, multiSelectTestOptions, MultiSelectOptions{Disabled: disabled, Label: "Flowers"}) + }) + _ = tt.Click("Before") + tt.Key(0, ui.KeyTab) + if !tt.Focused("Flowers") { + t.Fatal("field not focused") + } + disabled = true + tt.Key(0, ui.KeyBackspace) + if !slices.Equal(values, []int{1, 2}) { + t.Fatalf("Backspace removed a value: %v", values) + } +} + +func TestComboboxEnterAsItDisables(t *testing.T) { + value, disabled := "", false + tt := choiceBetween(func(c *ui.Context) { + Combobox(c, &value, comboboxTestFonts, ComboboxOptions{AllowCustom: true, Disabled: disabled, Label: "Font"}) + }) + _ = tt.Click("Before") + tt.Key(0, ui.KeyTab) + tt.Type("Caslon") + tt.Key(0, ui.KeyEscape) + disabled = true + tt.Key(0, ui.KeyEnter) + if value != "" { + t.Fatalf("Enter submitted %q", value) + } +} + +func TestCascaderRightAsRowDisables(t *testing.T) { + var path []string + f := newCascaderFake() + tt := ui.NewTester(func(c *ui.Context) { + Use(c, Settings{}) + Cascader(c, &path, CascaderOptions{Level: f.level, Load: f.load, AnyLevel: true, Label: "Place"}) + }, 700, 500) + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyDown) + if !tt.Focused("Europe") { + t.Fatal("Europe not focused") + } + roots := f.levels[""] + roots.Nodes = slices.Clone(roots.Nodes) + roots.Nodes[0].Disabled = true + f.levels[""] = roots + tt.Key(0, ui.KeyRight) + if len(path) != 0 || len(f.loads) != 0 { + t.Fatalf("Right on a disabled row chose %v, loaded %v", path, f.loads) + } +} + +func TestSelectEnterOnOptionAsItDisables(t *testing.T) { + v := "apple" + options := slices.Clone(selectTestOptions) + tt := ui.NewTester(func(c *ui.Context) { + Use(c, Settings{}) + Select(c, &v, options, SelectOptions{Label: "Fruit"}) + }, 400, 400) + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyDown) + tt.Key(0, ui.KeyDown) + options[1].Disabled = true + tt.Key(0, ui.KeyEnter) + if v != "apple" { + t.Fatalf("Enter chose the disabled option: %q", v) + } +} + +func TestEntryFocusSkipsDisabledFirstRow(t *testing.T) { + var path []string + f := &cascaderFake{levels: map[string]CascaderLevel{ + "": {Nodes: []CascaderNode{{Value: "aq", Label: "Antarctica", Disabled: true}, {Value: "eu", Label: "Europe", Leaf: true}}}, + }} + tt := ui.NewTester(func(c *ui.Context) { + Use(c, Settings{}) + Cascader(c, &path, CascaderOptions{Level: f.level, Load: f.load, Label: "Place"}) + }, 700, 500) + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyDown) + if !tt.Focused("Europe") { + t.Fatal("cascader entry did not reach the first enabled row") + } + nodes := []TreeSelectNode{{ID: "a", Label: "Archived", Disabled: true}, {ID: "b", Label: "Brass"}, {ID: "c", Label: "Copper"}} + var sel []string + tt = ui.NewTester(func(c *ui.Context) { + Use(c, Settings{}) + TreeSelect(c, &sel, nodes, TreeSelectOptions{Label: "Metal"}) + }, 500, 500) + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyEnter) + tt.Key(0, ui.KeyDown) + if !tt.Focused("Brass") { + t.Fatal("tree entry did not reach the first enabled row") + } + tt.Key(0, ui.KeyDown) + if !tt.Focused("Copper") { + t.Fatal("arrows stopped working after entry") + } +} diff --git a/combobox.go b/combobox.go index d970a51..934e46c 100644 --- a/combobox.go +++ b/combobox.go @@ -127,7 +127,10 @@ func Combobox(c *ui.Context, value *string, options []string, opts ComboboxOptio comboboxStatus(c, f.Bounds(), opts.Loading) } focused := in.Focused() - if v, ok := p.Chosen(); ok { + if in.IsDisabled() { + // Input delivered as the field turned disabled commits nothing. + *query = *value + } else if v, ok := p.Chosen(); ok { *value, *query = v, v *gen, *refocus = *gen+1, focused } else if in.Submitted() && opts.AllowCustom { diff --git a/docs/components/034-slider.md b/docs/components/034-slider.md index 2c26cec..97b36c8 100644 --- a/docs/components/034-slider.md +++ b/docs/components/034-slider.md @@ -45,5 +45,6 @@ ui.Render(view, 400, 300, 1) ## 限制 - 基于 `ui.SliderBase`，它向辅助技术报告 Slider 角色、范围与当前值（ui/base.go:128）。 -- MyGo 只把方向键、Home、End 交给 `SliderBase` 本身（ui/base.go:118，按键路由见 ui/input.go:495），并固定以范围的 1% 移动。本组件让它操作副本，按方向判断按键后自己按 `Step` 移动；距端点不足 1% 时 MyGo 的方向键与 Home/End 结果相同，组件改看松开的是哪个键（`HandleInput` 能收到 KeyUp），在键松开前不改值。 +- MyGo 只把方向键、Home、End 交给 `SliderBase` 本身（ui/base.go:118，按键路由见 ui/input.go:495），以范围的 1% 移动并把结果舍入到 1e-9（ui/widgets.go:375）。本组件让它操作副本，按方向判断按键后自己按 `Step` 移动。两种情况下按键结果无法区分：距端点不足 1% 时方向键与 Home/End 结果相同；步长细到 MyGo 舍入后不动。这时组件改看松开的是哪个键（`HandleInput` 能收到 KeyUp），在键松开时才改值；按住不放也只移动一步。 +- 步长网格复用 NumberInput 的实现：值为 `Min + n·Step`，小数位取 `Min` 与 `Step` 所需，端点跳转（End/Home、另一端）先对齐网格再写值并触发事件；例如 `Max=10, Step=3` 时 End 得到 9。 - `SliderBase` 在构建时就处理指针（ui/base.go:111），禁用时外层先禁用，且只操作副本，保证值不变。 diff --git a/examples/gallery/pages_choice.go b/examples/gallery/pages_choice.go index fbdd6a9..05adf56 100644 --- a/examples/gallery/pages_choice.go +++ b/examples/gallery/pages_choice.go @@ -37,7 +37,7 @@ items := []mujica.ToggleItem[string]{ view := func(c *ui.Context) { mujica.Use(c, mujica.Settings{}) mujica.ToggleGroup(c, &align, items, mujica.ToggleGroupOptions{Label: "Align"}) -}`, Keys: "Tab enters the group at the pressed item; Left/Right (Up/Down when vertical), Home and End move; Space presses."}, +}`, Keys: "Tab enters the group at the first enabled item, later at the last focused one; Left/Right (Up/Down when vertical), Home and End move; Space presses."}, {ID: "034", Name: "Slider", ZhName: "滑块", Category: choiceCategory, Demo: sliderDemo, Code: `volume := 40.0 view := func(c *ui.Context) { mujica.Use(c, mujica.Settings{}) diff --git a/multi_select.go b/multi_select.go index a0ca85d..188c76e 100644 --- a/multi_select.go +++ b/multi_select.go @@ -44,11 +44,11 @@ func MultiSelect[T comparable](c *ui.Context, values *[]T, options []SelectOptio *open = false } query := ui.Local(tr, multiSelectKey{"query"}, func() string { return "" }) - if tr.Clicked() || (!*open && tr.Shortcut(0, ui.KeyDown)) { + if tr.Clicked() || (!*open && tr.Shortcut(0, ui.KeyDown) && !tr.IsDisabled()) { *open = !*open *query = "" } - if n := len(*values); n > 0 && tr.Shortcut(0, ui.KeyBackspace) { + if n := len(*values); n > 0 && tr.Shortcut(0, ui.KeyBackspace) && !tr.IsDisabled() { *values = slices.Clone((*values)[:n-1]) } selectField(c, tr, *open, func() { diff --git a/select.go b/select.go index 78068ab..2be68a2 100644 --- a/select.go +++ b/select.go @@ -45,11 +45,18 @@ type selectTypeahead struct { func Select[T comparable](c *ui.Context, value *T, options []SelectOption[T], opts SelectOptions) ChoiceResult { k := Tokens(c) old := *value - value = choiceTarget(value, opts.Disabled) + // SelectBase chooses into a copy as it is built; a choice counts once the option is known enabled. + pick := old var s *ui.SelectParts[T] - // SelectBase takes clicks as it is built: a disabled wrapper and a copy block them. - ui.Row(c).Shrink(0).Disabled(opts.Disabled).Children(func() { s = ui.SelectBase(c, value) }) + ui.Row(c).Shrink(0).Disabled(opts.Disabled).Children(func() { s = ui.SelectBase(c, &pick) }) tr := s.Trigger + commit := func() { + if pick != *value && !tr.IsDisabled() && selectEnabled(options, pick) { + *value = pick + } + pick = *value + } + commit() if opts.Label != "" { tr.Label(opts.Label) } @@ -67,7 +74,7 @@ func Select[T comparable](c *ui.Context, value *T, options []SelectOption[T], op }) if ta.pending { ta.pending = false - if !s.Open() { + if !s.Open() && !tr.IsDisabled() { if i := selectFind(options, *value, ta.prefix); i >= 0 { *value = options[i].Value } @@ -75,7 +82,7 @@ func Select[T comparable](c *ui.Context, value *T, options []SelectOption[T], op } var zero T label, has := selectLabel(options, *value) - if opts.Clearable && has && (tr.Shortcut(0, ui.KeyBackspace) || tr.Shortcut(0, ui.KeyDelete)) { + if opts.Clearable && has && (tr.Shortcut(0, ui.KeyBackspace) || tr.Shortcut(0, ui.KeyDelete)) && !tr.IsDisabled() { *value, has = zero, false } selectField(c, tr, s.Open(), func() { @@ -88,6 +95,7 @@ func Select[T comparable](c *ui.Context, value *T, options []SelectOption[T], op *value = zero } }) + pick = *value lastHi := ui.Local(tr, selectKey{"shown"}, func() int { return -1 }) if opts.Disabled { // SelectParts has no way to close it (ui/base.go:310): a disabled select builds no popup. @@ -113,6 +121,7 @@ func Select[T comparable](c *ui.Context, value *T, options []SelectOption[T], op } }) }) + commit() if !s.Open() { *lastHi = -1 } @@ -123,6 +132,16 @@ func Select[T comparable](c *ui.Context, value *T, options []SelectOption[T], op return ChoiceResult{tr, changed} } +// selectEnabled reports whether v is an option that is not disabled. +func selectEnabled[T comparable](options []SelectOption[T], v T) bool { + for _, o := range options { + if o.Value == v { + return !o.Disabled + } + } + return false +} + // selectFind returns the enabled option matching typed prefix. func selectFind[T comparable](options []SelectOption[T], current T, prefix string) int { start := 0 diff --git a/slider.go b/slider.go index 1b31f97..3a546c8 100644 --- a/slider.go +++ b/slider.go @@ -40,9 +40,10 @@ const ( sliderMarkH float32 = 18 ) -// sliderGeom maps values to x positions across a slider's box. +// sliderGeom maps values to x positions across a slider's box; grid holds the step points. type sliderGeom struct { lo, hi, step float64 + grid numberInputSpec trackY float32 } @@ -51,23 +52,28 @@ func (g sliderGeom) x(r ui.Rect, v float64) float32 { return r.X + sliderThumb/2 + (r.W-sliderThumb)*max(0, min(1, f)) } -// snap rounds v to the nearest step inside the range. +// snap moves v to the nearest step point inside the range. func (g sliderGeom) snap(v float64) float64 { v = max(g.lo, min(g.hi, v)) if g.step <= 0 { return v } - v = g.lo + math.Round((v-g.lo)/g.step)*g.step - if v > g.hi { - v -= g.step - } - return math.Round(v*1e9) / 1e9 + return g.grid.stepBy(v, 0) } -// sliderKey is the last key released on a slider, and a move waiting for it. +// move goes n steps from v, 1% of the range each when continuous, staying in range. +func (g sliderGeom) move(v float64, n int) float64 { + if g.step <= 0 { + return max(g.lo, min(g.hi, v+float64(n)*(g.hi-g.lo)/100)) + } + return g.grid.stepBy(v, n) +} + +// sliderKey is the last key released on a slider, a move waiting for it, and whether its press moved. type sliderKey struct { up ui.Key pending int + acted bool } type sliderKeyID struct{} @@ -85,46 +91,69 @@ func sliderControl(c *ui.Context, v float64, g sliderGeom, from, to float64) (s ks.up = ev.Key return true }) - up, dir := ks.up, ks.pending + up, pending, acted := ks.up, ks.pending, ks.acted ks.up, ks.pending = 0, 0 + if up != 0 { + ks.acted = false + } if _, _, dragged = s.Dragged(); dragged { return s, max(from, min(to, g.snap(raw))), true } - unsure := dir != 0 + dir, jump, known := 0, false, true switch { - case raw < v: - dir, unsure = -1, false - case raw > v: - dir, unsure = 1, false + case raw != v: + // MyGo moved the copy 1% (an arrow) or to an end (Home, End), rounded to 1e-9 (ui/widgets.go:375). + dir = 1 + if raw < v { + dir = -1 + } + end := g.lo + if dir > 0 { + end = g.hi + } + arrow := max(g.lo, min(g.hi, v+float64(dir)*(g.hi-g.lo)/100)) + jump = math.Abs(raw-end) < math.Abs(raw-arrow) + if math.Abs(arrow-end) <= 1e-9+math.Abs(end)*1e-12 { + // Near an end an arrow and Home/End agree: the released key tells which it was. + dir, jump, known = sliderKeyMove(up, dir) + } + if up == 0 { + ks.acted = true + } + case pending != 0: + dir, jump, known = sliderKeyMove(up, pending) + case up != 0 && !acted: + // A step finer than MyGo's rounding leaves the copy still: act on the release. + dir, jump, known = sliderKeyMove(up, 0) } - if dir == 0 { + if !known { + ks.pending = dir return s, v, false } - end, limit := g.lo, from - if dir > 0 { - end, limit = g.hi, to + switch { + case dir == 0: + return s, v, false + case jump && dir < 0: + return s, g.snap(from), false + case jump: + return s, g.snap(to), false } - jump := raw == end - // An arrow within 1% of an end lands on it too: the released key tells which it was. - if unsure || jump && math.Abs(v-end) <= (g.hi-g.lo)/100*(1+1e-9) { - switch up { - case ui.KeyHome, ui.KeyEnd: - jump = true - case ui.KeyLeft, ui.KeyRight, ui.KeyUp, ui.KeyDown: - jump = false - default: - ks.pending = dir - return s, v, false - } + return s, max(from, min(to, g.move(v, dir))), false +} + +// sliderKeyMove reads a released key as direction and jump; an unknown key keeps dir, unresolved. +func sliderKeyMove(up ui.Key, dir int) (int, bool, bool) { + switch up { + case ui.KeyLeft, ui.KeyDown: + return -1, false, true + case ui.KeyRight, ui.KeyUp: + return 1, false, true + case ui.KeyHome: + return -1, true, true + case ui.KeyEnd: + return 1, true, true } - if jump { - return s, limit, false - } - step := g.step - if step <= 0 { - step = (g.hi - g.lo) / 100 - } - return s, max(from, min(to, g.snap(v+float64(dir)*step))), false + return dir, false, dir == 0 } func sliderSetup(min, max, step float64, marks []SliderMark, showValue bool, c *ui.Context) (sliderGeom, float32) { @@ -135,6 +164,10 @@ func sliderSetup(min, max, step float64, marks []SliderMark, showValue bool, c * panic("mujica: slider needs Min < Max and Step >= 0") } g := sliderGeom{lo: min, hi: max, step: step} + if step > 0 { + // NumberInput's grid: exact bounds and integer step indexes at the decimals Min and Step need. + g.grid = numberInputResolve(NumberInputOptions{Min: min, Max: max, Step: step, Precision: sliderDecimals(min, step)}) + } h := Density(c).ControlHeight() g.trackY = h / 2 if showValue { diff --git a/slider_grid_test.go b/slider_grid_test.go new file mode 100644 index 0000000..8915830 --- /dev/null +++ b/slider_grid_test.go @@ -0,0 +1,74 @@ +package mujica + +import ( + "testing" + + "github.com/egoist/mygo/ui" +) + +// sliderRecord builds a Slider and a RangeSlider and records the values each Changed reported. +type sliderRecord struct { + v, low, high float64 + single, lows, his []float64 +} + +func sliderGridTester(r *sliderRecord, min, max, step float64) *ui.Tester { + return ui.NewTester(func(c *ui.Context) { + Use(c, Settings{}) + ui.Column(c).Width(216).Gap(10).Children(func() { + if Slider(c, &r.v, SliderOptions{Min: min, Max: max, Step: step, Label: "One"}).Changed() { + r.single = append(r.single, r.v) + } + if RangeSlider(c, &r.low, &r.high, RangeSliderOptions{Min: min, Max: max, Step: step, Label: "Two"}).Changed() { + r.lows, r.his = append(r.lows, r.low), append(r.his, r.high) + } + }) + }, 300, 300) +} + +func TestSliderEndSnapsToStepGrid(t *testing.T) { + r := &sliderRecord{v: 3, low: 0, high: 3} + tt := sliderGridTester(r, 0, 10, 3) + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyEnd) + if len(r.single) != 1 || r.single[0] != 9 || r.v != 9 { + t.Fatalf("Slider End reported %v, value %v", r.single, r.v) + } + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyEnd) + if len(r.his) != 1 || r.his[0] != 9 || r.high != 9 { + t.Fatalf("RangeSlider End reported %v, high %v", r.his, r.high) + } +} + +func TestSliderTinyStep(t *testing.T) { + r := &sliderRecord{v: 0, low: 0, high: 1e-9} + tt := sliderGridTester(r, 0, 1e-9, 1e-10) + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyRight) + if r.v != 1e-10 { + t.Fatalf("Slider Right: %v", r.v) + } + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyRight) + if r.low != 1e-10 { + t.Fatalf("RangeSlider Right: %v", r.low) + } +} + +func TestSliderKeepsPreciseMin(t *testing.T) { + const lo = 0.1234567891 + r := &sliderRecord{v: 0.5, low: 0.5, high: 0.6} + tt := sliderGridTester(r, lo, 1, 0.1) + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyHome) + if r.v < lo || r.v != lo { + t.Fatalf("Slider Home: %v, Min %v", r.v, lo) + } + tt.Key(0, ui.KeyTab) + tt.Key(0, ui.KeyHome) + if r.low < lo || r.low != lo { + t.Fatalf("RangeSlider Home: %v, Min %v", r.low, lo) + } +} diff --git a/tree_select.go b/tree_select.go index 6194f48..e1e790c 100644 --- a/tree_select.go +++ b/tree_select.go @@ -74,7 +74,7 @@ func TreeSelect(c *ui.Context, selected *[]string, nodes []TreeSelectNode, opts if opts.Disabled { st.open = false } - if tr.Clicked() || (!st.open && tr.Shortcut(0, ui.KeyDown)) { + if tr.Clicked() || (!st.open && tr.Shortcut(0, ui.KeyDown) && !tr.IsDisabled()) { st.open, st.query = !st.open, "" } labels := map[string]string{} @@ -111,8 +111,9 @@ func TreeSelect(c *ui.Context, selected *[]string, nodes []TreeSelectNode, opts } ui.Scroll(c).MaxHeight(300).Padding(4).Children(func() { ui.Column(c).FocusGroup(ui.Vertical).Children(func() { + entry := slices.IndexFunc(rows, func(r treeSelectRow) bool { return !r.node.Disabled }) for i, r := range rows { - treeSelectRowView(c, st, selected, r, i == 0, shown, opts) + treeSelectRowView(c, st, selected, r, i == entry, shown, opts) } }) }) @@ -287,7 +288,7 @@ func treeSelectRowView(c *ui.Context, st *treeSelectState, selected *[]string, r b.Focus() st.focus = "" } - right, left := b.Shortcut(0, ui.KeyRight), b.Shortcut(0, ui.KeyLeft) + right, left := b.Shortcut(0, ui.KeyRight) && !b.IsDisabled(), b.Shortcut(0, ui.KeyLeft) && !b.IsDisabled() switch { case right && branch && !open: st.expanded[n.ID] = true