-
Notifications
You must be signed in to change notification settings - Fork 52
add keystrokes to run from canvas #3384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
02ce977
fc3e65f
1b719f9
4d3d316
6f5146e
7a7ac89
828dfac
ee31c6c
a88dc56
38e19a2
c392ea9
e33d70a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,7 +139,7 @@ function createKeyCombinationHook( | |
const target = e.target as HTMLElement; | ||
const focusedScope = | ||
target | ||
?.closest('[data-keybinding-scope]') | ||
.closest('[data-keybinding-scope]') | ||
?.getAttribute('data-keybinding-scope') || null; | ||
|
||
const keyMatchingHandlers = Array.from(keyHandlers).filter(h => | ||
|
@@ -160,7 +160,7 @@ function createKeyCombinationHook( | |
|
||
const maxPriority = Math.max(...matchingHandlers.map(h => h.priority)); | ||
const topPriorityHandlers = matchingHandlers.filter( | ||
h => h.priority === maxPriority | ||
h => h.priority === maxPriority as PriorityLevel | ||
); | ||
|
||
// Take the last handler if there are more than one with the same priority. | ||
|
@@ -259,10 +259,12 @@ const submitAction = (_e: KeyboardEvent, el: HTMLElement) => { | |
/** | ||
* Simulates a "close" action, used to close modals, panels, or other UI components. | ||
* | ||
* @param e - The keyboard event that triggered the action. | ||
* @param _e - The keyboard event that triggered the action. | ||
* @param el - The DOM element associated with the hook. | ||
*/ | ||
const closeAction = (_e: KeyboardEvent, el: HTMLElement) => el.click(); | ||
const closeAction = (_e: KeyboardEvent, el: HTMLElement) => { | ||
el.click(); | ||
}; | ||
|
||
/** | ||
* Hook to trigger a form submission when "Ctrl+S" (or "Cmd+S" on macOS) is pressed. | ||
|
@@ -366,3 +368,84 @@ export const CloseNodePanelViaEscape = createKeyCombinationHook( | |
closeAction, | ||
PRIORITY.NORMAL | ||
); | ||
|
||
/** | ||
* Handles Ctrl+Enter to trigger run actions directly based on current state. | ||
* | ||
* BEHAVIOR (based purely on URL state): | ||
* 1. If in inspector (URL contains 'm=expand'): | ||
* → Click #save-and-run button to execute the workflow | ||
* 2. If in run panel (URL contains 'm=workflow_input'): | ||
* → Click #run-from-input-selector button to execute the workflow | ||
* 3. If step selected but not in inspector (URL contains 's=' but no 'm=expand'): | ||
* → Click the appropriate run button (#run-from-step or #run-from-trigger) | ||
* 4. If no step selected and no panel: | ||
* → Click #run-from-top button to run from trigger | ||
*/ | ||
const openRunPanelAction = () => { | ||
const url = window.location.href; | ||
|
||
// Only work on workflow pages | ||
if (!url.includes('/w/')) { | ||
return; | ||
} | ||
|
||
// Parse URL state | ||
const hasStepSelected = url.includes('s='); | ||
const isInInspector = url.includes('m=expand'); | ||
const isInRunPanel = url.includes('m=workflow_input'); | ||
Comment on lines
+394
to
+396
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we've ever made keyHandlers depend on url params. It's mostly dependent on element that are available when we're on the screen we want the key events to be active. eg. Undo/Redo Key handlers are registered when the workflowDiagram is mounted and unregistered when it's unmounted. Hence we wouldn't have to depend on query params. |
||
|
||
if (isInInspector) { | ||
// Inspector mode - click the save-and-run button | ||
const runButton = document.querySelector('#save-and-run:not([disabled])'); | ||
if (runButton instanceof HTMLElement) { | ||
runButton.click(); | ||
} | ||
} else if (isInRunPanel) { | ||
// Run panel mode - click the run-from-input-selector button | ||
const runButton = document.querySelector('#run-from-input-selector:not([disabled])'); | ||
if (runButton instanceof HTMLElement) { | ||
runButton.click(); | ||
} | ||
} else if (hasStepSelected) { | ||
// Step selected but not in inspector - click the appropriate run button | ||
// Try run-from-step first (for jobs), then run-from-trigger (for triggers) | ||
const runFromStepButton = document.querySelector('#run-from-step:not([disabled])'); | ||
if (runFromStepButton instanceof HTMLElement) { | ||
runFromStepButton.click(); | ||
} else { | ||
const runFromTriggerButton = document.querySelector('#run-from-trigger:not([disabled])'); | ||
if (runFromTriggerButton instanceof HTMLElement) { | ||
runFromTriggerButton.click(); | ||
} | ||
} | ||
} else { | ||
// No step selected - trigger the same navigation as the run button | ||
const runButton = document.querySelector('#run-from-top:not([disabled])'); | ||
if (runButton instanceof HTMLElement) { | ||
runButton.click(); | ||
} | ||
Comment on lines
+398
to
+427
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using ids here is brittle. we might want to actually call events directly when they exist. |
||
} | ||
}; | ||
|
||
/** | ||
* Hook to open the Run panel when "Ctrl+Enter" (or "Cmd+Enter" on macOS) is pressed. | ||
* | ||
* This hook is scoped to the workflow editor and navigates to the run panel URL, which opens the | ||
* workflow input interface for running the workflow. | ||
* | ||
* Priority: `PRIORITY.HIGH` within its scope, ensuring it takes precedence over other handlers in the workflow editor. | ||
* Scope: `"workflow-editor"`, meaning this hook only applies within the workflow editor context. | ||
*/ | ||
export const OpenRunPanelViaCtrlEnter = createKeyCombinationHook( | ||
(e) => { | ||
console.log('OpenRunPanelViaCtrlEnter: Key check triggered'); | ||
return isCtrlOrMetaEnter(e); | ||
}, | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
(_e, _el) => { | ||
console.log('OpenRunPanelViaCtrlEnter: Action triggered'); | ||
openRunPanelAction(); | ||
}, | ||
PRIORITY.HIGH | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -213,8 +213,12 @@ defmodule LightningWeb.WorkflowLive.Edit do | |
class="transition-all duration-300 ease-in-out" | ||
/> | ||
<div | ||
class={"relative h-full flex grow transition-all duration-300 ease-in-out #{if @show_new_workflow_panel, do: "w-2/3", else: ""}"} | ||
class={"relative h-full flex grow transition-all duration-300 ease-in-out #{if @show_new_workflow_panel, do: "w-2/3", else: ""} focus:outline-none"} | ||
id={"workflow-edit-#{@workflow.id}"} | ||
phx-hook="OpenRunPanelViaCtrlEnter" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This keyhandler registration doesn't seem to get unregistered. |
||
data-keybinding-scope="workflow-editor" | ||
tabindex="0" | ||
phx-mounted={JS.focus()} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wondering why we need to focus here. |
||
> | ||
<.selected_template_label | ||
:if={@selected_template && @show_new_workflow_panel} | ||
|
@@ -225,13 +229,17 @@ defmodule LightningWeb.WorkflowLive.Edit do | |
<div class="flex-none" id="job-editor-pane"> | ||
<div | ||
:if={@selected_job && @selection_mode == "expand"} | ||
id={"inspector-panel-#{@selected_job.id}"} | ||
class={[ | ||
"fixed left-0 top-0 right-0 bottom-0 flex-wrap", | ||
"hidden opacity-0", | ||
"bg-white inset-0 z-30 overflow-hidden drop-shadow-[0_35px_35px_rgba(0,0,0,0.25)]" | ||
"bg-white inset-0 z-30 overflow-hidden drop-shadow-[0_35px_35px_rgba(0,0,0,0.25)] focus:outline-none" | ||
]} | ||
phx-mounted={fade_in()} | ||
phx-remove={fade_out()} | ||
data-keybinding-scope="workflow-editor" | ||
tabindex="0" | ||
phx-hook="AutoFocusOnMount" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another focusing of the inspector. wondering once again. |
||
> | ||
<LightningWeb.WorkflowLive.JobView.job_edit_view | ||
job={@selected_job} | ||
|
@@ -505,6 +513,7 @@ defmodule LightningWeb.WorkflowLive.Edit do | |
patch={"#{@base_url}?s=#{@selected_job.id}&m=workflow_input"} | ||
type="button" | ||
theme="primary" | ||
id={"run-from-step"} | ||
> | ||
Run | ||
</.button_link> | ||
|
@@ -590,6 +599,7 @@ defmodule LightningWeb.WorkflowLive.Edit do | |
patch={"#{@base_url}?s=#{@selected_trigger.id}&m=workflow_input"} | ||
type="button" | ||
theme="primary" | ||
id="run-from-trigger" | ||
> | ||
<.icon name="hero-play-solid" class="w-4 h-4" /> Run | ||
</.button_link> | ||
|
@@ -3163,6 +3173,7 @@ defmodule LightningWeb.WorkflowLive.Edit do | |
patch={"#{@base_url}?s=#{@trigger_id}&m=workflow_input"} | ||
type="button" | ||
theme="primary" | ||
id="run-from-top" | ||
> | ||
Run | ||
</.button_link> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To run the workflow from the run-panel. we don't need to search for the run button using selectors which will be problematic with time. We seem to already have a function in the js world that does that. It'll be best if we can register our key events close to this function and then just call it directly instead of depending on searching for a button.