Make processor description clickable to edit#271219
Conversation
nikita-lavrov
added
release_note:skip
nikita-lavrov
added
Team:obs-onboarding
|
Pinging @elastic/obs-onboarding-team (Team:obs-onboarding) |
| } | ||
| display="block" | ||
| > | ||
| <EuiText |
There was a problem hiding this comment.
question: if this text is working as a button should it have a aria-label for accessibility? not really sure if it works with the EuiText even with the role="button"
There was a problem hiding this comment.
In 5353ca1 I changed it to EuiEmptyButton instead. The Voiceover correctly announces the tooltip content when focusing on the button.
| <i>{stepDescription}</i> | ||
| </p> | ||
| <p> | ||
| {i18n.translate( |
There was a problem hiding this comment.
This edit text is rendering unconditionally, even when the user has no permissions to edit
|
|
||
| const saveDescription = () => { | ||
| const trimmedEditValue = editValue.trim(); | ||
| if (stepDescription !== trimmedEditValue) { |
There was a problem hiding this comment.
possible edge case: if the user wants to freeze the auto-generated description it needs to edit and then edit again to do it, instead of comparing to the stepDescription that is the display name we could check against the step.description. I am not sure if this will really be something our users want to do but I don't see any downside in checking against the real description, wdyt?
There was a problem hiding this comment.
I think this would be confusing for the user, as in this case simply interacting with the auto-generated description will prompt the user to save the changes, even though from their perspective they haven't changed anything.
My rationale is that the description saved in a processor definition is a custom description, whereas the one we show the user in this case is a default one, that the user can change if they want to. Does this make sense?
There was a problem hiding this comment.
Nice work here @nikita-lavrov , really smooth to edit now! Just found a small bug that needs fixing, if you have a processor saved in a stream and edit it, then edit the description and clear it (to revert to the previous name) when you press enter the Unsaved chip from the processor is gone, the button to Save is still present so I'm not sure if the changes are really gone or if it's just the chip
Also left some comments but those are small/picky stuff
| return !isEqual( | ||
| sanitiseForEditing(nextStreamlangDSL), | ||
| sanitiseForEditing(previousStreamlangDSL) |
There was a problem hiding this comment.
StreamlangDSL might have the same object and properties, but their order might be different. This causes the equality check to fail when we convert them to strings.
| const actionRestSanitised = stripUndefinedValues(actionRest); | ||
| return removeCustomIdentifiers | ||
| ? actionRestSanitised | ||
| : { ...actionRestSanitised, customIdentifier }; |
There was a problem hiding this comment.
This prevents the bottom bar from appearing after we reverted all the changes we've made.
This works because when we save processor changes, the state machine receives an action step that can contain undefined values (those that we left empty while editing the processor). Because these undefined values aren't present in the previousStreamlangDSL, the equality check fails.
I can extract this and the changes in the state machines into a separate PR if there are concerns.
Fixed in 5353ca1 |
💔 Build Failed
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
History
|
3 participants
Summary
Closes https://github.com/elastic/streams-program/issues/1198
This PR makes the processor description clickable to edit.
It also changes the behavior of the
Unsavedbadge. Now it only shows if the description, parent or branch of the processor differs from its original state. This prevents the badge from being displayed after we reverted the changes that we didn't save.Demo
Before
Screen.Recording.2026-05-26.at.10.58.46.AM.mov
After
Screen.Recording.2026-05-26.at.10.47.09.AM.mov