AI Asisstant: Add a notice that the workflow is in live mode - #5205
josephjclark wants to merge 8 commits into
Conversation
|
All changes are frontend TypeScript/React additions: a draft-mode banner in the AI assistant panel, a new Security Review ✅
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5205 +/- ##
=======================================
- Coverage 91.2% 91.2% -0.0%
=======================================
Files 453 453
Lines 22844 22844
=======================================
- Hits 20828 20826 -2
- Misses 2016 2018 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Will review this today @josephjclark |
lmac-1
left a comment
There was a problem hiding this comment.
Thanks for picking this up Joe. I did some testing today in Tyrell's session and agree that this is a very confusing experience right now 😅. We definitely need to let the user know that changes are not applied when the workflow is live.
I had a question about the approach before the details. The banner is always showing when a workflow is live, but some users might only open AI assistant to ask a question. This banner looks like something is "wrong" and guides users to switch to draft which would "disable" the workflow. Is there a way for us to flag it on the message instead (if it tries to apply an edit)? Could say something like "Not applied, this workflow is live. Switch to draft". Then the warning only appears when something was actually lost.
If we do go with the banner, a few small things:
- Could the button say "Switch to draft"? That's what the dialog and header says and I think we should stick to the same language (comment applied inline)
- Do we need
useSwitchToDraft? It's the same two lines asuseWorkflowActions().switchToDraftand the panel renders insideAIAssistantPanelWrapperwhich is already withinLiveViewActionsProvider. Can we not passswitchToDraftdown as a prop? - What about using
role="status"rather than alert and also an info icon rather than the lock?
| </div> | ||
| )} | ||
|
|
||
| <AlertDialog |
There was a problem hiding this comment.
Is this a second instance of the same alert dialog? Can we not make this one component?
|
@lmac-1 Thanks for the review! I've addressed most comments.
I think this sound complicated? We'd have to look at the response and maybe make the alert flash up.... and anyway it's kinda too late as the user has asked for the change. I best 80% of AI usage is for edits anyway. I appreciate that this here isn't a nice solution. It's ugly and inelegant, and not even terribly clear. Then when you hit "switch to draft" you're then told that the workflow will be taken offline. I mean it's scary and horrible. I just want a quick fix in so that we have some coverage on this. I'm surprised we haven't had reports that the assistant is broken as a result of this 🤔 Just trying to get ahead of that. |
lmac-1
left a comment
There was a problem hiding this comment.
Makes sense! Thanks for addressing most of the feedback. I did 2 direct commits to clean up two other parts that still refer to 'draft mode' (comment + error toast message) which I forgot to flag in my last review. Approved
Draft mode introduces quite thorny UX problem for the AI Assistant.
The Assistant cannot edit a workflow while a workflow is live. It'll try to, and claim it's made changes. But they won't actually apply. Users must remember to enter draft mode first.
This PR adds a quick, cheap fix: a little banner which reminds users that a workflow is live and cannot be edited/
This affects users with experimental features switched on.
The banner disappears if draft mode is enabled.
The banner does not appear to users with experimental features switched off.
I think this needs patching out.
AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy
Pre-submission checklist
/reviewwith Claude Code)
(e.g.,
:owner,:admin,:editor,:viewer)