Skip to content
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

Fix/partial run #164

Merged
merged 6 commits into from
Feb 18, 2025
Merged

Fix/partial run #164

merged 6 commits into from
Feb 18, 2025

Conversation

srijanpatel
Copy link
Collaborator

@srijanpatel srijanpatel commented Feb 18, 2025

This pull request includes significant updates to the Header and RunModal components in the frontend, with a focus on state management improvements and code simplification. The most important changes include the introduction of new state variables for managing the selected test input and the run modal's open state, as well as the refactoring of related components and hooks to use these new state variables.

State Management Improvements:

  • Added selectedTestInputId and isRunModalOpen state variables to the flowSlice to manage the selected test input and the run modal's open state (frontend/src/store/flowSlice.ts) [1] [2].

Component Updates:

  • Updated Header component to use the new selectedTestInputId and isRunModalOpen state variables, replacing local state variables and simplifying modal handling (frontend/src/components/Header.tsx) [1] [2] [3] [4] [5] [6].
  • Refactored RunModal component to remove selectedRow state and use selectedTestInputId from the global state instead (frontend/src/components/modals/RunModal.tsx) [1] [2] [3] [4] [5] [6] [7] [8] [9].

Hook Updates:

  • Modified usePartialRun hook to leverage the new selectedTestInputId state and handle the case where no initial inputs are provided by opening the run modal (frontend/src/hooks/usePartialRun.ts) [1] [2] [3] [4].

These changes collectively enhance the maintainability and readability of the codebase by centralizing state management and reducing redundancy.


Important

Improve state management in Header and RunModal by introducing global state variables for selected test input and run modal state.

  • State Management:
    • Add selectedTestInputId and isRunModalOpen to flowSlice for managing selected test input and run modal state.
  • Component Updates:
    • Update Header to use selectedTestInputId and isRunModalOpen, replacing local state variables.
    • Refactor RunModal to remove selectedRow state, using selectedTestInputId from global state.
  • Hook Updates:
    • Modify usePartialRun to use selectedTestInputId and handle no initial inputs by opening run modal.
  • Misc:
    • Add setSelectedTestInputId and setRunModalOpen actions in flowSlice.

This description was created by Ellipsis for 748dbe2. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 748dbe2 in 1 minute and 45 seconds

More details
  • Looked at 439 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 10 drafted comments based on config settings.
1. frontend/src/components/Header.tsx:87
  • Draft comment:
    Good refactor: using global state (setRunModalOpen) to control the modal improves consistency.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. frontend/src/components/modals/RunModal.tsx:77
  • Draft comment:
    Transition from local selectedRow state to global selectedTestInputId is well-managed; ensure the useEffect dependencies include dispatch.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. frontend/src/hooks/usePartialRun.ts:45
  • Draft comment:
    The fallback for initialInputs using the selected test input is clear and handles the case when no inputs are provided by opening the modal.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. frontend/src/store/flowSlice.ts:757
  • Draft comment:
    New reducers (setSelectedTestInputId and setRunModalOpen) are added properly and integrated within the state. This centralizes state management for UI modals.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. frontend/src/types/api_types/flowStateSchema.ts:21
  • Draft comment:
    The FlowState type now includes selectedTestInputId and isRunModalOpen, ensuring type safety for global state.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
6. frontend/src/components/Header.tsx:49
  • Draft comment:
    Remove unused local state 'isDebugModalOpen' since global 'isRunModalOpen' (from the store) is now used. This will simplify the component.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. frontend/src/components/modals/RunModal.tsx:513
  • Draft comment:
    Consider extracting the repeated condition checking for unsaved editor content into a constant to improve readability.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
8. frontend/src/hooks/usePartialRun.ts:46
  • Draft comment:
    The logic for deriving 'effectiveInitialInputs' from selectedTestInputId and testInputs looks good. Consider adding a brief comment to explain this fallback for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 30% <= threshold 50%
    None
9. frontend/src/store/flowSlice.ts:650
  • Draft comment:
    When deleting a test input, consider also resetting 'selectedTestInputId' if it matches the deleted input, to keep the global state consistent.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. frontend/src/types/api_types/flowStateSchema.ts:21
  • Draft comment:
    FlowState schema now includes selectedTestInputId and isRunModalOpen. The changes look consistent with the new global state management. Make sure all components using these state variables have been updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50%
    None

Workflow ID: wflow_iK0updgwkauU0akw


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@srijanpatel srijanpatel merged commit 12487e3 into main Feb 18, 2025
1 check passed
@srijanpatel srijanpatel deleted the fix/partialRun branch February 18, 2025 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant