-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Streams] [Streamlang] Enable nested conditionals in the UI #232322
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
Conversation
481cc46 to
a8e4687
Compare
eaf285b to
5571a49
Compare
fb540ff to
daf1811
Compare
..._app/public/components/data_management/stream_detail_enrichment/steps/blocks/where/index.tsx
Outdated
Show resolved
Hide resolved
...app/public/components/data_management/stream_detail_enrichment/steps/blocks/action/index.tsx
Outdated
Show resolved
Hide resolved
|
If the condition preview is too long, it destroys the layout:
It seems like I can't reorder the blocks at all? That seems too much - I know we said reordering across layers is not important, but I don't think we can't take away reordering all together. Was this discussed? Does not have to addressed on this PRSome things that stood out to me - don't have to be addressed right now, but probably worth discussing / noting down for later. When using conditionals, we still always calculate the grok preview clientside, even though it's not applied to all samples:
We should probably do something about it, but I'm not sure what. I don't like the idea of also simulating the conditional logic client side, that seems like a slippery slope. Maybe we can check whether the processor ran and use that to enable/disable the client side preview or something like that? Delete button gets very big - does this make sense? Every action (adding a processor/condition, editing a processor/condition, deleting a processor/condition) is two clicks away - this feels cumbersome, why don't we give the user these buttons directly? In practice this will speed up editing by a lot: |
|
@tonyghiani Thanks for the review 👌
Good catch, I'll bring back the tooltip.
That's fair, I'll change the UX there. ✅
Hmm, yeah, the processor one was full length at one point, it's predominantly condition edit states modelled in the designs. I'll change it. ✅
Will look into that one. ✅
Agreed, will change that 👍 ✅
Thought I'd added truncation everywhere it was needed, I'll fix it. ✅ Removed the text truncation component as it doesn't fulfill our needs. Used the suggestions here.
Yeah, this was discussed in one of the sync meetings with Shay, Dima etc. The takeaway was that if we can't do it right (waiting for the new DnD from EUI) then just remove it.
Yeah, probably needs addressing in a followup. Will have a think through solutions.
Yeah, same as Marco's feedback, something that changed over time. ✅
I think this was just a case of trying to provide maximum real estate to summaries etc. @boriskirov what do you think? |
- Change default condition to ALWAYS condition (stops immediate form error) - Put border on panels - Amend delete processor button width
…into 445-nested-conditionals
tonyghiani
left a comment
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.
Finished a first round on the code as well, it was a very nice read 👏
I left minor nits around, no major blockers, let me know if you have any question!
.../data_management/stream_detail_enrichment/state_management/simulation_state_machine/types.ts
Show resolved
Hide resolved
...nents/data_management/stream_detail_enrichment/state_management/steps_state_machine/utils.ts
Show resolved
Hide resolved
...ugins/shared/streams_app/public/components/data_management/stream_detail_enrichment/utils.ts
Outdated
Show resolved
Hide resolved
x-pack/platform/packages/shared/kbn-streamlang/src/transpilers/shared/convert_for_ui.ts
Outdated
Show resolved
Hide resolved
...richment/state_management/stream_enrichment_state_machine/stream_enrichment_state_machine.ts
Show resolved
Hide resolved
...richment/state_management/stream_enrichment_state_machine/stream_enrichment_state_machine.ts
Show resolved
Hide resolved
...s/data_management/stream_detail_enrichment/state_management/use_steps_processing_summary.tsx
Outdated
Show resolved
Hide resolved
...omponents/data_management/stream_detail_enrichment/steps/blocks/action/processor_metrics.tsx
Outdated
Show resolved
Hide resolved
...app/public/components/data_management/stream_detail_enrichment/steps/blocks/context_menu.tsx
Outdated
Show resolved
Hide resolved
- Remove duplicate event types - Improve UI step conversion functions
|
@tonyghiani Thanks for the thorough reviewing 🙏 I think everything is addressed now. I've updated #232322 (comment) with ✅ statuses and notes. And addressed the inline feedback. Just a couple of notes:
Left this for now because, unless I'm missing something, this is no different to
I'll take a look in a followup. I really just wanted to keep the UI properties and the "pure" definition separate. Copy amendments can come in a followup. Scout test expansion can also come in a followup. I'm very cognizant of the fact this is blocking several issues also targeted at 9.2, so let me know if there's anything left you see as a blocker 👌 |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Public APIs missing exports
Unknown metric groupsAPI count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
cc @Kerry350 |
|
Thanks for addressing the changes! The only issue I still found was switching branches. I got the following issue:
Hopefully, is just a local error to me, but could you please double-check if we get any error on the following workflow:
|
|
@tonyghiani I can't seem to replicate with a totally fresh setup going that way ( |
That's what happened then, cause I switched and didn't have the latest commits yet, thanks for validating! |
tonyghiani
left a comment
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.
It looks good to me, thanks for this incredible piece of work! 👏
The follow-ups are defined, please double-check with @flash1293 if he has any other concern/comment before merging, as we reviewed this in parallel.
florent-leborgne
left a comment
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.
Copy/API docs LGTM!
…232322) ## Summary Closes elastic/streams-program#445 This PR adds support for nested conditionals (nested `where` blocks in Streamlang DSL) to the processing UI. There have been architectural changes to the state machines, and UI changes to match the [Figma](https://www.figma.com/design/C4o0y9Knk4jYXjsulvbW3w/Stream-Management--Processors--Grok?node-id=6004-42432&p=f&t=QSrMxFyNLwrO2mLq-0) designs. ## States There are quite a lot of states within the UI, I've tried to capture the majority of them here. Also be mindful of UX additions such as scrolling newly added steps into view. <details> <summary>Screenshots</summary> <img width="1186" height="1216" alt="Screenshot 2025-09-11 at 22 17 48" src="https://github.com/user-attachments/assets/5ea4e45f-4398-4463-b5e5-81d8be005555" /> <img width="1164" height="1182" alt="Screenshot 2025-09-11 at 22 18 40" src="https://github.com/user-attachments/assets/a5604248-926c-49ec-bd00-18cae7a8aece" /> <img width="1174" height="508" alt="Screenshot 2025-09-11 at 22 18 49" src="https://github.com/user-attachments/assets/fbe19f40-761b-46fe-a2ac-6ad2a3795043" /> <img width="1150" height="246" alt="Screenshot 2025-09-11 at 22 19 05" src="https://github.com/user-attachments/assets/75f80061-985e-4600-871a-abda65d2f0d0" /> <img width="1148" height="1062" alt="Screenshot 2025-09-11 at 22 19 43" src="https://github.com/user-attachments/assets/78353291-809e-47a7-bb1c-76995559c38b" /> <img width="1176" height="664" alt="Screenshot 2025-09-11 at 22 20 10" src="https://github.com/user-attachments/assets/a92feeac-d0b4-43a8-8e44-42f34aea3a24" /> <img width="1260" height="380" alt="Screenshot 2025-09-11 at 22 20 17" src="https://github.com/user-attachments/assets/b9063711-58a0-41c1-8644-c876909734a6" /> <img width="1166" height="874" alt="Screenshot 2025-09-11 at 22 20 31" src="https://github.com/user-attachments/assets/7f593938-e447-4e68-a260-3bf39ff6af8f" /> </details> ## On the way - ~Existing Scout tests are still being updated (this doesn't need to block reviews)~ - Update Scout tests to cover new features - Copy needs review --------- Co-authored-by: kibanamachine <[email protected]>
## Summary Closes elastic/streams-program#445 This PR adds support for nested conditionals (nested `where` blocks in Streamlang DSL) to the processing UI. There have been architectural changes to the state machines, and UI changes to match the [Figma](https://www.figma.com/design/C4o0y9Knk4jYXjsulvbW3w/Stream-Management--Processors--Grok?node-id=6004-42432&p=f&t=QSrMxFyNLwrO2mLq-0) designs. ## States There are quite a lot of states within the UI, I've tried to capture the majority of them here. Also be mindful of UX additions such as scrolling newly added steps into view. <details> <summary>Screenshots</summary> <img width="1186" height="1216" alt="Screenshot 2025-09-11 at 22 17 48" src="https://github.com/user-attachments/assets/5ea4e45f-4398-4463-b5e5-81d8be005555" /> <img width="1164" height="1182" alt="Screenshot 2025-09-11 at 22 18 40" src="https://github.com/user-attachments/assets/a5604248-926c-49ec-bd00-18cae7a8aece" /> <img width="1174" height="508" alt="Screenshot 2025-09-11 at 22 18 49" src="https://github.com/user-attachments/assets/fbe19f40-761b-46fe-a2ac-6ad2a3795043" /> <img width="1150" height="246" alt="Screenshot 2025-09-11 at 22 19 05" src="https://github.com/user-attachments/assets/75f80061-985e-4600-871a-abda65d2f0d0" /> <img width="1148" height="1062" alt="Screenshot 2025-09-11 at 22 19 43" src="https://github.com/user-attachments/assets/78353291-809e-47a7-bb1c-76995559c38b" /> <img width="1176" height="664" alt="Screenshot 2025-09-11 at 22 20 10" src="https://github.com/user-attachments/assets/a92feeac-d0b4-43a8-8e44-42f34aea3a24" /> <img width="1260" height="380" alt="Screenshot 2025-09-11 at 22 20 17" src="https://github.com/user-attachments/assets/b9063711-58a0-41c1-8644-c876909734a6" /> <img width="1166" height="874" alt="Screenshot 2025-09-11 at 22 20 31" src="https://github.com/user-attachments/assets/7f593938-e447-4e68-a260-3bf39ff6af8f" /> </details> ## On the way - ~Existing Scout tests are still being updated (this doesn't need to block reviews)~ - Update Scout tests to cover new features - Copy needs review --------- Co-authored-by: kibanamachine <[email protected]>
…232322) ## Summary Closes elastic/streams-program#445 This PR adds support for nested conditionals (nested `where` blocks in Streamlang DSL) to the processing UI. There have been architectural changes to the state machines, and UI changes to match the [Figma](https://www.figma.com/design/C4o0y9Knk4jYXjsulvbW3w/Stream-Management--Processors--Grok?node-id=6004-42432&p=f&t=QSrMxFyNLwrO2mLq-0) designs. ## States There are quite a lot of states within the UI, I've tried to capture the majority of them here. Also be mindful of UX additions such as scrolling newly added steps into view. <details> <summary>Screenshots</summary> <img width="1186" height="1216" alt="Screenshot 2025-09-11 at 22 17 48" src="https://github.com/user-attachments/assets/5ea4e45f-4398-4463-b5e5-81d8be005555" /> <img width="1164" height="1182" alt="Screenshot 2025-09-11 at 22 18 40" src="https://github.com/user-attachments/assets/a5604248-926c-49ec-bd00-18cae7a8aece" /> <img width="1174" height="508" alt="Screenshot 2025-09-11 at 22 18 49" src="https://github.com/user-attachments/assets/fbe19f40-761b-46fe-a2ac-6ad2a3795043" /> <img width="1150" height="246" alt="Screenshot 2025-09-11 at 22 19 05" src="https://github.com/user-attachments/assets/75f80061-985e-4600-871a-abda65d2f0d0" /> <img width="1148" height="1062" alt="Screenshot 2025-09-11 at 22 19 43" src="https://github.com/user-attachments/assets/78353291-809e-47a7-bb1c-76995559c38b" /> <img width="1176" height="664" alt="Screenshot 2025-09-11 at 22 20 10" src="https://github.com/user-attachments/assets/a92feeac-d0b4-43a8-8e44-42f34aea3a24" /> <img width="1260" height="380" alt="Screenshot 2025-09-11 at 22 20 17" src="https://github.com/user-attachments/assets/b9063711-58a0-41c1-8644-c876909734a6" /> <img width="1166" height="874" alt="Screenshot 2025-09-11 at 22 20 31" src="https://github.com/user-attachments/assets/7f593938-e447-4e68-a260-3bf39ff6af8f" /> </details> ## On the way - ~Existing Scout tests are still being updated (this doesn't need to block reviews)~ - Update Scout tests to cover new features - Copy needs review --------- Co-authored-by: kibanamachine <[email protected]>









Summary
Closes https://github.com/elastic/streams-program/issues/445
This PR adds support for nested conditionals (nested
whereblocks in Streamlang DSL) to the processing UI. There have been architectural changes to the state machines, and UI changes to match the Figma designs.States
There are quite a lot of states within the UI, I've tried to capture the majority of them here. Also be mindful of UX additions such as scrolling newly added steps into view.
Screenshots
On the way
Existing Scout tests are still being updated (this doesn't need to block reviews)