Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Improve trigger inspector UX#4854

Open
midigofrank wants to merge 14 commits into
main from
4787-new-trigger-inspector-flow
Open

Improve trigger inspector UX #4854
midigofrank wants to merge 14 commits into
main from
4787-new-trigger-inspector-flow

Conversation

@midigofrank

@midigofrank midigofrank commented Jun 15, 2026
edited
Loading

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.
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 Bot commented Jun 15, 2026
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.5%. Comparing base (84c0754) to head (e6065ba).

Additional details and impacted files
@@ Coverage Diff @@
## main #4854 +/- ##
=====================================
 Coverage 90.5% 90.5% 
=====================================
 Files 445 445 
 Lines 22721 22721 
=====================================
+ Hits 20560 20561 +1 
+ Misses 2161 2160 -1 

☔ 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

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.

Comment thread assets/js/collaborative-editor/components/inspector/trigger/WebhookUrlField.tsx Outdated
Comment thread assets/js/collaborative-editor/components/inspector/trigger/WebhookUrlField.tsx Outdated
Comment thread assets/js/collaborative-editor/components/inspector/trigger/useTriggerDraft.ts Outdated
Comment thread assets/js/collaborative-editor/components/inspector/TriggerInspector.tsx Outdated
});
// 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.

midigofrank reacted with thumbs up emoji
@github-project-automation github-project-automation Bot moved this from New Issues to In review in Core Jun 15, 2026
- Move CronFieldBuilder and its test into trigger/; only the cron
 Configure step uses it
- Replace hardcoded hex in WebhookUrlField with Tailwind emerald classes
- Trim trigger/index.ts to the exports TriggerInspector actually consumes
- Extract the duplicated sameIdSet into trigger/idSet.ts
- Disable WebhookConfigureStep inputs when the workflow is read-only, to
 match the cron and kafka Configure steps
- Drop the speculative typeless-trigger path from TriggerInspector and the
 wizard's initial step
- Align Kafka Configure input borders with the webhook convention
- Lowercase "On a schedule" for casing consistency with "On webhook call"
Diff the draft against an open-time baseline snapshot rather than the
live trigger, so Finish writes only the fields the user actually changed.
Comparing against the live trigger treated a collaborator's concurrent
edit to an untouched field (e.g. toggling enabled on the canvas) as a
local edit and reverted it on Finish; isDirty had the same false positive.
Also commit the auth-method change before the local trigger write so a
failed channel request leaves nothing partially committed and reports
failure to keep the wizard open.
Fold WebhookChooseStep into TriggerChooseStep: the two were ~80%
identical, differing only in the webhook URL field and exit affordance
(footer Cancel vs. header back arrow). The merged step takes `type` plus
the webhook-only URL props, and chooses its exit shape from whether
`onBack` or `onCancel` is passed.
Replace the three near-identical wizard footers (WizardFinishFooter plus
the inline footers in the webhook Choose/Configure steps) with a single
WizardFooter, parameterized by primary label, optional Cancel, and an
optional validation error. This also removes the inconsistency where the
webhook Configure step hand-rolled its footer instead of sharing one.
No behavior change; the trigger inspector test suite is unchanged and
passes.
Drop comments that restated the adjacent code and metadata that no
longer aids the reader — Figma version references, issue-number tags,
and "ported from / former TriggerForm" historical notes. Keep per-prop
JSDoc, the functional doc blocks, and the rationale comments that
explain non-obvious behaviour (draft baseline and auth-load guards,
the server-side auth-modal relay, enabled-state preservation).

Copy link
Copy Markdown
Collaborator Author

Thanks @lmac-1 , I have incorporated your changes, except for making global components. My intentions with these changes were to reuse existing components and not introduce new ones, I think this can come in as a follow up. Would you mind giving it another go?

lmac-1 reacted with thumbs up emoji lmac-1 reacted with heart emoji

@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.

Looks good Frank, thanks for applying the changes. I would feel more comfortable if another dev could give this a quickish glance/review before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lmac-1 lmac-1 lmac-1 approved these changes

@doc-han doc-han Awaiting requested review from doc-han

At least 2 approving reviews are required to merge this pull request.

Labels

None yet

Projects

Status: In review

Milestone

No milestone

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

AltStyle によって変換されたページ (->オリジナル) /