Skip to content

Improve trigger inspector UX#4854

Open
midigofrank wants to merge 9 commits into
mainfrom
4787-new-trigger-inspector-flow
Open

Improve trigger inspector UX#4854
midigofrank wants to merge 9 commits into
mainfrom
4787-new-trigger-inspector-flow

Conversation

@midigofrank

@midigofrank midigofrank commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR redesigns the trigger inspector, replacing the old edit-in-place form with a read-only resting panel and a guided edit wizard, for webhook, cron, and kafka triggers.

Closes #4787
Closes #4797
Closes #4798

What changed for users:

Previously, selecting a trigger node dropped you straight into an editable form that wrote every change live, field by field. Now selecting a trigger shows a read-only "resting" panel that summarises its configuration, with an Edit button. Editing opens a wizardChoose → (optionally Change the type via a picker) → Configure → Finish — and changes are held in a local draft, only persisted when you click Finish. Cancel discards them. So editing a trigger is now a deliberate, reviewable action rather than a live mutation.

The same flow applies to every trigger type:

  • Webhook — resting panel shows the URL (with copy), authentication methods, and a response summary; the wizard configures the response type (immediate vs. on-completion + custom status codes) and authentication (a row-based credential picker, with "create new" via the existing modal). Inline links on the resting panel ("Add authentication" / "Configure default response status") deep-link straight into the relevant Configure section.
  • Cron — resting panel shows the humanised schedule and input source; the wizard uses the frequency builder plus the cron input source selector.
  • Kafka — resting panel summarises hosts/topics/SSL/auth; the wizard ports the full connection/security/advanced configuration.

The resting panels are read-only by design and drop the Enabled toggle and Run button — enabling/disabling lives on the canvas, and running lives in the header.

No backend changes — trigger schema and persistence are unchanged; this is a frontend/UX redesign in the collaborative editor.

Validation steps

  1. Open a workflow and select a webhook trigger node → confirm the read-only resting panel (URL + copy, authentication, response summary). Click Edit → step through Choose → Configure, change the response type and/or attach an auth method, click Finish → the resting panel reflects the change. Repeat and click Cancel → no change is persisted.
  2. From the webhook resting panel, click the inline "Add authentication" / "Configure default response status" links → confirm they open the wizard directly on Configure with the right section expanded.
  3. Select a cron trigger → resting panel shows the humanised schedule; Edit → change the frequency → Finish → schedule updates.
  4. Click Edit → Change to open the type picker, pick a different type, then Cancel → confirm the trigger type is not changed (the switch is held in the draft until Finish).
  5. With kafka_triggers_enabled, repeat for a kafka trigger (connection/security/advanced config, validation messages).
  6. As a viewer / on a read-only workflow → confirm the Edit button is disabled with an explanatory tooltip.

Additional notes for the reviewer

  1. Draft/commit model is intentional. The wizard deliberately diverges from the rest of the editor's live per-field sync: edits buffer in a local draft and commit in one shot on Finish. Commit writes only the fields that actually changed, so a collaborator's concurrent edit to an untouched field isn't clobbered.
  2. The kafka option only appears in the type picker when kafka_triggers_enabled is set; kafka is covered by unit tests and was browser-verified by temporarily forcing the flag on.
  3. This redesign retires TriggerForm and WebhookAuthMethodModal (and their tests). The unified wizard lives in assets/js/collaborative-editor/components/inspector/trigger/.

AI Usage

Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our
Responsible AI Policy

Pre-submission checklist

  • I have performed an AI review of my code (we recommend using /review
    with Claude Code)
  • I have implemented and tested all related authorization policies.
    (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

Rework the webhook trigger flow (#4787) across the show panel and the
Choose/Configure wizard:

- Add a flat ghost button variant; move shadow out of the shared base so
  ghost buttons have no raised outline.
- Picker: drop the "Popular" badge, add a trailing chevron per row.
- Choose: show the trigger type as a chip with a "Change" button that opens
  the picker.
- Configure: replace the native Response Type select with a Listbox carrying
  per-option descriptions; collapse Authentication and Response Options by
  default; let users type any Success/Error status code (default 201); drop
  the header back arrow for a clickable breadcrumb; label the commit button
  "Finish".
- Show panel: use a secondary Edit button, drop the section separators, and
  summarise the response in one line with a link to the docs.

Authentication still uses the existing modal and is unchanged here.
Resting panel:
- Make Authentication and Response collapsible, with an at-a-glance
  auth count in the header ("none configured" / "N configured")
- Show the response type description and, when responding on
  completion, the default success/error statuses as read-only inputs
  with a note that job code can override them
- Replace prominent buttons with inline links ("Add authentication",
  "Configure default response status") that deep-link straight into
  the relevant Configure section

Edit wizard:
- Replace the multi-select credential dropdown with a row-per-method
  picker: "Add" appends a row, each row dedupes against the others,
  and a "Create a new authentication method" link opens the form
- Hide Response Options unless the webhook responds on completion
- Relay the create-auth modal's close event so Cancel works in the
  new flow
Bring the cron trigger onto the redesigned inspector flow, matching the
webhook pattern: a read-only show panel (humanized schedule + cron input
source) and a Choose -> Configure edit wizard over a local draft that
commits on Finish.

Configure reuses the CronFieldBuilder dropdown for the schedule, which
now auto-opens the raw cron expression field when Custom is selected and
shows a plain-English description of the expression beneath it.

Extract the shared TriggerTypeStep so the webhook and cron wizards no
longer duplicate the picker step wiring.
Replace the per-type WebhookEditWizard and CronEditWizard with a single
TriggerEditWizard that owns the draft and step machine and dispatches the
Choose and Configure steps by trigger type. The initial step is
`type ? 'choose' : 'picker'`, so supporting a nullable type later is a
one-line change.

Port kafka onto the new flow (KafkaConfigureStep + KafkaShowPanel,
generalized TriggerChooseStep) so the legacy TriggerForm — and its
now-orphaned WebhookAuthMethodModal — can be deleted. Kafka drops the
Enabled toggle and Run button to match the webhook and cron panels.

Guard useWebhookTrigger so it only requests trigger auth methods for
webhook triggers, avoiding needless channel calls for cron and kafka.
@midigofrank midigofrank self-assigned this Jun 15, 2026
@github-project-automation github-project-automation Bot moved this to New Issues in Core Jun 15, 2026
Fold TriggerTypeStep back into TriggerPicker and route all three trigger
types through the draft path, so picking a type holds until Finish — Cancel
now reverts a kafka type switch instead of committing it immediately. Commit
only the trigger fields that actually changed so a concurrent canvas edit to
an untouched field is no longer clobbered, and stop a type change from
silently re-enabling a disabled trigger. Resync the webhook auth-method rows
when the selection resolves asynchronously.

Extract shared EditFooter, ReadOnlyField, WizardFinishFooter and
useCanEditWorkflow used across the show panels and configure steps, and add a
trigger/ barrel for external consumers.

Share the trigger test harness across the inspector test files, add a
WebhookConfigureStep test, and collapse the repetitive cron and inspector
suites with test.each.
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.5%. Comparing base (817e445) to head (2ee916e).

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4854     +/-   ##
=======================================
- Coverage   90.5%   90.5%   -0.0%     
=======================================
  Files        445     445             
  Lines      22721   22721             
=======================================
- Hits       20561   20555      -6     
- Misses      2160    2166      +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@midigofrank midigofrank marked this pull request as ready for review June 15, 2026 05:33
@midigofrank midigofrank requested review from doc-han and lmac-1 June 15, 2026 05:33
@github-actions

Copy link
Copy Markdown

Confirmed: no backend changes. The PR is a pure frontend refactor of the trigger inspector that calls pre-existing, already-secured Phoenix channel handlers (request_trigger_auth_methods / update_trigger_auth_methods in lib/lightning_web/channels/workflow_channel.ex:507,538 — both scope on socket.assigns.project.id and gate writes with authorize_edit_workflow).

Security Review ✅

  • S0 (project scoping): N/A — no backend changes; the new TypeScript hooks call existing channel events (request_trigger_auth_methods, update_trigger_auth_methods) whose handlers already scope queries to socket.assigns.project.id (lib/lightning_web/channels/workflow_channel.ex:516-523,549-553).
  • S1 (authorization): N/A — no new web-layer actions; the unchanged update_trigger_auth_methods handler still gates on authorize_edit_workflow(socket) (workflow_channel.ex:549) before any mutation.
  • S2 (audit trail): N/A — no new config-resource writes from server code; the existing handler still passes actor: socket.assigns.current_user into WebhookAuthMethods.update_trigger_auth_methods (workflow_channel.ex:558) which produces the audit entry.

@lmac-1 lmac-1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great, Frank! I have left some comments in the code re organisation and permissions. I do think some of these things need cleaning up before merging, so that we are in a great place for further design system work.

As a side conversation, as we are moving into a newer design system, I am wondering whether we should have some of these newer 'primitive' components in one place? e.g. breadcrumbs, buttons, badges. Then, as new epics come in, we can start to use these components rather than the legacy ones. Thoughts @theroinaochieng @stuartc ? Not for this PR, but something to clean up in future work or as a follow-on PR.


Some design niggles (not to block approval - perhaps something for Tyrell to review later in UAT, but just noting for reference):

Image 1. "On webhook call" vs "On a Schedule" - I feel like "Schedule" should be lower case so that it's consistent casing. Image 2. This lighter purple for "Change" gave me a feeling that it was inactive or disabled because of the lighter colour.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this inside /trigger too? Or is this shared with other inspector components?

border-gray-200 p-2"
>
<span
className="shrink-0 rounded bg-[#eafaf3] px-2 py-1 font-mono

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid hardcoded hex tailwind classes? As we're moving to a design system, please put these in tailwind.config.ts for easy reference

Something like this under theme.extend.colors:

extend: {
    colors: {
      'brand-green': {
        light: '#eafaf3',
        DEFAULT: '#006840',
      },    
    },
  }

Don't worry too much about the naming, we can always rename when Tyrell confirms the design system/colours, but good to have in one place. Then you can swap the hardcoded values for bg-brand-green and bg-brand-green-light

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Goodness me, great catch @lmac-1

>
<span
className="shrink-0 rounded bg-[#eafaf3] px-2 py-1 font-mono
text-[10px] font-medium leading-none text-[#006840]"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here re hardcoded hex

createDefaultTrigger('kafka') as Extract<Workflow.Trigger, { type: 'kafka' }>
).kafka_configuration;

const inputClass = cn(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same pattern exists in WebhookConfigureStep and WebhookShowPanel but with slight inconsistencies — this file uses rounded-md and border-slate-300 while the webhook files use rounded-lg and border-gray-200. Should these be consistent and aligned?

(Not necessary for approval, just a bit of housekeeping)

import { WebhookAuthMethodSelect } from './WebhookAuthMethodSelect';
import { WizardBreadcrumb } from './WizardBreadcrumb';

const codeInputClass = cn(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on KafkaConfigureStep

}

/** Order-independent comparison of two id sets. */
function sameIdSet(a: string[], b: string[]): boolean {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is also in WebhookAuthMethodSelect, can we move it out to a shared utils or somewhere where both reference the same function?

}

/** Order-independent comparison of two id sets. */
function sameIdSet(a: string[], b: string[]): boolean {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment in useTriggerDraft

: 'Enable or disable this trigger';
// A typeless trigger has no show panel; route it straight to the wizard,
// which starts at its picker so the user can choose a type.
if (view === 'edit' || !trigger.type) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If !trigger.type is ever true for a read-only user (e.g. starting from a blank canvas later on), they'll land in the edit wizard with no permission check. WebhookConfigureStep also doesn't call useWorkflowReadOnly unlike its cron/kafka siblings. Worth keeping in mind as the trigger system evolves.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example of a primitive component from the new design system that could sit outside of /trigger. Not sure where though, and could be in a follow-up PR.

});
// Each row: [description, triggerType, extra trigger fields, expected heading]
test.each<[string, string, Record<string, unknown>, string]>([
[

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No read-only scenario tested here — worth adding a case with can_edit_workflow: false to cover the !trigger.type path mentioned in TriggerInspector comment.

@github-project-automation github-project-automation Bot moved this from New Issues to In review in Core Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

New trigger flow: Update trigger New trigger flow: Create show/resting page for trigger inspector Epic: Better trigger flow

2 participants