Skip to content

Commit e10b980

Browse files
authored
[Bug Fix] Prevent Command dialog stacking (#386)
* [Bug Fix] Prevent Command dialog stacking (#230) * docs(command): note single-instance dialog behavior * refactor(command): track open dialog via static instance ref Replace querySelector + hardcoded selector with a static `openInstance` reference set in connect() and cleared in disconnect(). Removes the `data-ruby-ui--command-dialog` marker attribute and per-controller DOM lookups, so renaming the controller or restructuring dialog markup no longer silently breaks single-instance behavior. Addresses review feedback on #386. * refactor(command): split dialog controller, use Stimulus outlets Introduce ruby-ui--command-dialog controller for the trigger/template wrapper and keep ruby-ui--command for the cloned dialog instance. The trigger declares a ruby-ui--command outlet matched by a marker attribute on the cloned dialog wrapper. Outlet connect/disconnect callbacks track the active dialog, replacing the static class field and avoiding both querySelector and same-identifier dual-personality controller code. - New: command_dialog_controller.js (trigger + content target + outlet) - Strip open/openValue/content target from command_controller.js - Rename trigger actions to ruby-ui--command-dialog#open - Add ruby_ui__command_dialog_instance marker on cloned dialog
1 parent 013c1fd commit e10b980

10 files changed

Lines changed: 96 additions & 57 deletions

File tree

docs/app/javascript/controllers/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ application.register("ruby-ui--combobox", RubyUi__ComboboxController)
4646
import RubyUi__CommandController from "./ruby_ui/command_controller"
4747
application.register("ruby-ui--command", RubyUi__CommandController)
4848

49+
import RubyUi__CommandDialogController from "./ruby_ui/command_dialog_controller"
50+
application.register("ruby-ui--command-dialog", RubyUi__CommandDialogController)
51+
4952
import RubyUi__ContextMenuController from "./ruby_ui/context_menu_controller"
5053
application.register("ruby-ui--context-menu", RubyUi__ContextMenuController)
5154

docs/app/javascript/controllers/ruby_ui/command_controller.js

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,7 @@ import Fuse from "fuse.js";
33

44
// Connects to data-controller="ruby-ui--command"
55
export default class extends Controller {
6-
static targets = ["input", "group", "item", "empty", "content"];
7-
8-
static values = {
9-
open: {
10-
type: Boolean,
11-
default: false,
12-
},
13-
};
6+
static targets = ["input", "group", "item", "empty"];
147

158
connect() {
169
this.selectedIndex = -1;
@@ -22,24 +15,6 @@ export default class extends Controller {
2215
this.inputTarget.focus();
2316
this.searchIndex = this.buildSearchIndex();
2417
this.toggleVisibility(this.emptyTargets, false);
25-
26-
if (this.openValue && this.hasContentTarget) {
27-
this.open();
28-
}
29-
}
30-
31-
open(e) {
32-
if (e) {
33-
e.preventDefault();
34-
}
35-
36-
if (!this.hasContentTarget) {
37-
return;
38-
}
39-
40-
document.body.insertAdjacentHTML("beforeend", this.contentTarget.innerHTML);
41-
// prevent scroll on body
42-
document.body.classList.add("overflow-hidden");
4318
}
4419

4520
dismiss() {
@@ -49,6 +24,10 @@ export default class extends Controller {
4924
this.element.remove();
5025
}
5126

27+
focusInput() {
28+
this.inputTarget?.focus();
29+
}
30+
5231
filter(e) {
5332
// Deselect any previously selected item
5433
this.deselectAll();
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { Controller } from "@hotwired/stimulus";
2+
3+
// Connects to data-controller="ruby-ui--command-dialog"
4+
export default class extends Controller {
5+
static targets = ["content"];
6+
static outlets = ["ruby-ui--command"];
7+
8+
rubyUiCommandOutletConnected(controller) {
9+
this.openOutlet = controller;
10+
}
11+
12+
rubyUiCommandOutletDisconnected() {
13+
this.openOutlet = null;
14+
}
15+
16+
open(e) {
17+
if (e) {
18+
e.preventDefault();
19+
}
20+
21+
if (!this.hasContentTarget) {
22+
return;
23+
}
24+
25+
if (this.openOutlet) {
26+
this.openOutlet.focusInput();
27+
return;
28+
}
29+
30+
document.body.insertAdjacentHTML("beforeend", this.contentTarget.innerHTML);
31+
// prevent scroll on body
32+
document.body.classList.add("overflow-hidden");
33+
}
34+
}

docs/app/views/docs/command.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ def view_template
9393
RUBY
9494
end
9595

96+
Heading(level: 2) { "Single instance" }
97+
98+
p(class: "text-muted-foreground") do
99+
plain "The Command dialog is single-instance. Activating a trigger while the dialog is already open refocuses the existing dialog instead of stacking another one on top, so repeated keybindings or trigger clicks behave predictably."
100+
end
101+
96102
render Components::ComponentSetup::Tabs.new(component_name: component)
97103

98104
render Docs::ComponentsTable.new(component_files(component))

gem/lib/ruby_ui/command/command_controller.js

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,7 @@ import Fuse from "fuse.js";
33

44
// Connects to data-controller="ruby-ui--command"
55
export default class extends Controller {
6-
static targets = ["input", "group", "item", "empty", "content"];
7-
8-
static values = {
9-
open: {
10-
type: Boolean,
11-
default: false,
12-
},
13-
};
6+
static targets = ["input", "group", "item", "empty"];
147

158
connect() {
169
this.selectedIndex = -1;
@@ -22,24 +15,6 @@ export default class extends Controller {
2215
this.inputTarget.focus();
2316
this.searchIndex = this.buildSearchIndex();
2417
this.toggleVisibility(this.emptyTargets, false);
25-
26-
if (this.openValue && this.hasContentTarget) {
27-
this.open();
28-
}
29-
}
30-
31-
open(e) {
32-
if (e) {
33-
e.preventDefault();
34-
}
35-
36-
if (!this.hasContentTarget) {
37-
return;
38-
}
39-
40-
document.body.insertAdjacentHTML("beforeend", this.contentTarget.innerHTML);
41-
// prevent scroll on body
42-
document.body.classList.add("overflow-hidden");
4318
}
4419

4520
dismiss() {
@@ -49,6 +24,10 @@ export default class extends Controller {
4924
this.element.remove();
5025
}
5126

27+
focusInput() {
28+
this.inputTarget?.focus();
29+
}
30+
5231
filter(e) {
5332
// Deselect any previously selected item
5433
this.deselectAll();

gem/lib/ruby_ui/command/command_dialog.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ def view_template(&)
1010

1111
def default_attrs
1212
{
13-
data: {controller: "ruby-ui--command"}
13+
data: {
14+
controller: "ruby-ui--command-dialog",
15+
ruby_ui__command_dialog_ruby_ui__command_outlet: "[data-ruby-ui--command-dialog-instance]"
16+
}
1417
}
1518
end
1619
end

gem/lib/ruby_ui/command/command_dialog_content.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ def initialize(size: :md, **attrs)
1717
end
1818

1919
def view_template(&block)
20-
template(data: {ruby_ui__command_target: "content"}) do
21-
div(data: {controller: "ruby-ui--command"}) do
20+
template(data: {ruby_ui__command_dialog_target: "content"}) do
21+
div(data: {controller: "ruby-ui--command", ruby_ui__command_dialog_instance: true}) do
2222
backdrop
2323
div(**attrs, &block)
2424
end
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { Controller } from "@hotwired/stimulus";
2+
3+
// Connects to data-controller="ruby-ui--command-dialog"
4+
export default class extends Controller {
5+
static targets = ["content"];
6+
static outlets = ["ruby-ui--command"];
7+
8+
rubyUiCommandOutletConnected(controller) {
9+
this.openOutlet = controller;
10+
}
11+
12+
rubyUiCommandOutletDisconnected() {
13+
this.openOutlet = null;
14+
}
15+
16+
open(e) {
17+
if (e) {
18+
e.preventDefault();
19+
}
20+
21+
if (!this.hasContentTarget) {
22+
return;
23+
}
24+
25+
if (this.openOutlet) {
26+
this.openOutlet.focusInput();
27+
return;
28+
}
29+
30+
document.body.insertAdjacentHTML("beforeend", this.contentTarget.innerHTML);
31+
// prevent scroll on body
32+
document.body.classList.add("overflow-hidden");
33+
}
34+
}

gem/lib/ruby_ui/command/command_dialog_trigger.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class CommandDialogTrigger < Base
88
].freeze
99

1010
def initialize(keybindings: DEFAULT_KEYBINDINGS, **attrs)
11-
@keybindings = keybindings.map { |kb| "#{kb}->ruby-ui--command#open" }
11+
@keybindings = keybindings.map { |kb| "#{kb}->ruby-ui--command-dialog#open" }
1212
super(**attrs)
1313
end
1414

@@ -21,7 +21,7 @@ def view_template(&)
2121
def default_attrs
2222
{
2323
data: {
24-
action: ["click->ruby-ui--command#open", @keybindings.join(" ")]
24+
action: ["click->ruby-ui--command-dialog#open", @keybindings.join(" ")]
2525
}
2626
}
2727
end

gem/test/ruby_ui/command_test.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,6 @@ def test_render_with_all_items
6060
end
6161

6262
assert_match(/Search/, output)
63+
assert_match(/data-controller="ruby-ui--command"/, output)
6364
end
6465
end

0 commit comments

Comments
 (0)