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

Auto minimize the sidebar in workflow editor #848

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

ykeremy
Copy link
Contributor

@ykeremy ykeremy commented Sep 18, 2024

<!-- ELLIPSIS_HIDDEN -->
🚀 This description was created by Ellipsis for commit 5bee7b8

feat: auto minimize sidebar in workflow editor using SidebarStore

Summary:

Introduces SidebarStore for managing sidebar state and updates components to auto-collapse the sidebar.

Key points:

  • Introduces SidebarStore for managing sidebar state.
  • Updates RootLayout to use useSidebarStore for collapsed state.
  • Adjusts sidebar width and icon logic in RootLayout.
  • Sets collapsed to true in WorkflowEditor on mount using useMountEffect.

Generated with ❤️ by ellipsis.dev

…src/'

<!-- ELLIPSIS_HIDDEN -->

| 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit f55115a63cacc22b25ff0052ac08cadb3b716010  |
|--------|--------|

feat: auto minimize sidebar in workflow editor using SidebarStore

### Summary:
Introduces `SidebarStore` for managing sidebar state and updates `RootLayout` and `WorkflowEditor` to use it, auto-collapsing the sidebar in the workflow editor.

**Key points**:
- **State Management**:
  - Introduces `SidebarStore` in `SidebarStore.ts` using `zustand` to manage sidebar collapsed state.
- **RootLayout Component**:
  - Replaces local `useState` with `useSidebarStore` for `collapsed` state.
  - Updates sidebar width and icon logic to use `collapsed` from `SidebarStore`.
- **WorkflowEditor Component**:
  - Uses `useSidebarStore` to set `collapsed` to `true` on mount using `useMountEffect`.

----
Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev)

<!-- ELLIPSIS_HIDDEN -->
@ykeremy ykeremy added the sync label Sep 18, 2024
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 5bee7b8 in 8 seconds

More details
  • Looked at 52 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/WorkflowEditor.tsx:41
  • Draft comment:
    Consider using useEffect directly instead of useMountEffect for better clarity and standard practice.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of useMountEffect is not standard and might be confusing. It's better to use useEffect directly for clarity.
2. skyvern-frontend/src/routes/workflows/editor/WorkflowEditor.tsx:33
  • Draft comment:
    Simplify the retrieval of setCollapsed.
const setCollapsed = useSidebarStore(state => state.setCollapsed);
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The setCollapsed function is being retrieved in a slightly verbose way. It can be simplified.

Workflow ID: wflow_YkjJrzIrmq8DIZhT


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

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! Incremental review on 5bee7b8 in 14 seconds

More details
  • Looked at 52 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/WorkflowEditor.tsx:41
  • Draft comment:
    Consider using useEffect directly instead of useMountEffect for better clarity and standard practice.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of useMountEffect is not standard and might be confusing. It's better to use useEffect directly for clarity.
2. skyvern-frontend/src/store/SidebarStore.ts:10
  • Draft comment:
    If the sidebar should always start collapsed in the workflow editor, consider initializing collapsed to true to avoid flicker on mount.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The setCollapsed function is being used correctly, but the initial state of collapsed is set to false in SidebarStore.ts. This might cause a flicker effect when the component mounts and immediately sets it to true. Consider initializing it to true if the sidebar should always start collapsed in the workflow editor.

Workflow ID: wflow_SqBiwo5cVvcFvQgq


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

@msalihaltun msalihaltun merged commit 8f04efa into main Sep 18, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/auto-minimize-sidebar-in-editor branch September 18, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants