Skip to content

Refactor form navigation to left panel #724

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

Merged

Conversation

ohltyler
Copy link
Member

@ohltyler ohltyler commented May 14, 2025

Description

Refactors the workflow detail page, such that users click on a component in the new left nav panel, and configure in the central panel. All components (ingest and search) are viewable altogether from the new left nav. The same guardrails are in place, such that users cannot make changes to the search flow if there are unsaved changes to ingest, and vice versa.

Main component changes:

  • LeftNav: the base component with all of the components
  • IngestContent: the accordion of all of the ingest-flow-related components
  • SearchContent: the accordion of all of the search-flow-related components
  • ComponentInput: the rough "replacement" of WorkflowInputs. Dynamically renders based on the selected component from the left nav. Shows the relevant form input for such selected component.
  • ProcessorsComponent / ProcessorList: refactoring of the existing ProcessorsList, as this functionality has now moved as nav components within LeftNav.

Other implementation details:

  • adds disabled prop to most form input-related components. This is used to still allow users to drill down and view all form details, even for components that are readonly, if the opposite context (ingest or search) has unsaved changes.
  • moves input-related components from workflow_inputs dir => component_input
  • removes the save/undo buttons from the workflow detail header, and all related form state props. This simplifies things and moves all form actions into one place, at the bottom of the left nav. Consequentially, this cleans up the parent WorkflowDetail component, as all complex state has moved into the child ResizableWorkspace component, as props are not needed to be shared to WorkflowDetailHeader anymore.
  • adds general NavComponent, a reusable component for creating each of the clickable cards from the left nav.
  • enhancement: don't set default search pipeline on an index. Instead, just conditionally execute searches with/without the pipeline.
  • bug fix: filter out workflows of type REGISTER_AGENTS, as currently there is no native support of this use case on the UI. In the future, we may support agent creation/configuration for different chatbot-related use cases.

Next steps for future PRs to this feature branch, before merging back into main:

  • further testing / verifying existing use cases
  • formatting/spacing/sizing of all components
  • passing UT/IT

Demo video showing the new pattern, new form states, and overall user flow:

left-nav-demo.webm

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

ohltyler added 30 commits May 14, 2025 12:33
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
@ohltyler ohltyler marked this pull request as draft May 15, 2025 00:10
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
@ohltyler ohltyler marked this pull request as ready for review May 15, 2025 14:36
ohltyler added 15 commits May 15, 2025 09:23
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
… resp processors

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
… params;

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
@kamahuan
Copy link
Contributor

Hey Tyler, I downloaded the code and noticed a few issues. Please correct me if my understanding is not right:
image

  1. If the ingest flow has any errors, the mockup says ingest flow will roll back to Not created state, but the UI shows active state.
  2. Wording on the right panel under Inspect is inconsistent with the mockup.
  3. test flow tab on the right panel always has a run test button and the UI shows more context than the mockup.
  4. The icon on the upper top left panel seems different from the mockup.

Another problem is that should we add a button in the sample data section so that user can delete the sample data if they change their mind. Currently there is no button to roll back to the initial state.

image The second screenshots show I created ingest sample data and create the ingest pipeline, and then change to another sample data and config the processor, do we need `create ingest pipeline` button in this case? I can see the `update` button shows if I make any changes to the processor. The red box is inconsistent wording and icon. image The third image shows that if the user clicked any component, it will show `selected`. Did not see the same `selected` text on the mockup. Tried click `Retrive from data source` after search pipeline created but returned nothing. Is this behavior expected. image Another issue is that when there is unsaved changes, the mockup shows a callout instead of the `update` button.

@ohltyler
Copy link
Member Author

Thanks for taking a look. As a reminder, this is to get the bulk of the functionality into the feature branch. The final styling/wording/spacing etc is not finalized yet.

If the ingest flow has any errors, the mockup says ingest flow will roll back to Not created state, but the UI shows active state.

It greatly depends on what the error is. If it is an error creating, it will stay in not created state. If it is an error running ingest, it will still be active, as it just means there was some processor causing some failure and should be updated, but the resources are still active/created. This is the expected behavior.

Wording on the right panel under Inspect is inconsistent with the mockup.

We can ignore final details/wording at this stage, this will be done in later UX review finalization stages.

test flow tab on the right panel always has a run test button and the UI shows more context than the mockup.

This is the expected behavior, mockups may not show everything.

The icon on the upper top left panel seems different from the mockup.

Same comment as previous, this will all be finalized later on. I chose this one over the mockup due to sizing issues currently.

The second screenshots show I created ingest sample data and create the ingest pipeline, and then change to another sample data and config the processor, do we need create ingest pipeline button in this case? I can see the update button shows if I make any changes to the processor.

Yes, because if any changes to the ingest pipeline, the data needs to be re-ingested to be processed correctly.

The red box is inconsistent wording and icon.

This is a moving target from UX end and still being finalized, current implementation matches previous mocks. This like above callouts around small wording/styling will be finalized later on.

The third image shows that if the user clicked any component, it will show selected. Did not see the same selected text on the mockup. Tried click Retrive from data source after search pipeline created but returned nothing. Is this behavior expected.

Same comments as above, this is still being finalized.

Another issue is that when there is unsaved changes, the mockup shows a callout instead of the update button.

Same comments as above, this is still being finalized.

ohltyler added 3 commits May 22, 2025 08:01
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
@saimedhi
Copy link
Collaborator

Hi Tyler, thank you for the PR! I'll start reviewing it now.

ohltyler added 3 commits May 22, 2025 09:10
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Copy link
Collaborator

@saimedhi saimedhi left a comment

Choose a reason for hiding this comment

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

@ohltyler, overall looks great to me. This PR looks good to be merged into feature branch.

I will do more testing in all different scenarios and let you know if I find any bug. Overall nav_components and component_input code looks clean

@ohltyler ohltyler merged commit 746d95d into opensearch-project:feature/editor-refactor May 22, 2025
2 of 3 checks passed
@ohltyler ohltyler deleted the left-panel-1 branch May 22, 2025 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature A new feature for the plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants