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

feat(platform): update Combobox to add new workspace to state #796

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

Allan2000-Git
Copy link
Contributor

@Allan2000-Git Allan2000-Git commented Feb 24, 2025

User description

Description

This PR adds newly created workspace to existing list of workspaces and modified background color of create workspace dialog to be consistent with other create dialog components.

Fixes #666

Dependencies

N/A

Future Improvements

N/A

Mentions

@rajdip-b

Screenshots of relevant screens

https://www.loom.com/share/d37bab94864e4d229c55026df679304a

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

PR Type

Bug fix, Enhancement


Description

  • Fixes issue where newly created workspace doesn't appear in the dropdown.

  • Updates the workspace state to include the newly created workspace.

  • Changes the background color of the create workspace dialog for consistency.


Changes walkthrough 📝

Relevant files
Enhancement
combobox.tsx
Fix workspace dropdown and enhance dialog UI                         

apps/platform/src/components/ui/combobox.tsx

  • Adds the newly created workspace to the list of workspaces.
  • Updates the background color of the create workspace dialog.
  • Ensures the dropdown reflects the updated workspace state.
  • +2/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    666 - PR Code Verified

    Compliant requirements:

    • After creating a new workspace, it should appear in the workspace dropdown list
    • The workspace dropdown should update automatically after successful creation

    Requires further human verification:

    • The workspace creation flow should work end-to-end (needs manual testing to verify the full flow)
    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The success toast is shown before checking if the data exists. Consider moving the toast.success call inside the data existence check.

    if (success && data) {
      toast.success('Workspace created successfully')
      setSelectedWorkspace({ ...data, projects: 0 })

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add error handling for failures

    Add error handling for the case when workspace creation fails. Currently, the
    success toast is shown before checking if data exists.

    apps/platform/src/components/ui/combobox.tsx [75-80]

     if (success && data) {
    -  toast.success('Workspace created successfully')
       setSelectedWorkspace({ ...data, projects: 0 })
       setAllWorkspaces((prev) => [...prev, { ...data, projects: 0 }])
       setOpen(false)
    +  toast.success('Workspace created successfully')
    +} else {
    +  toast.error('Failed to create workspace')
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: Adding error handling with user feedback for failed workspace creation is crucial for a better user experience and helps users understand when operations fail. The suggestion also correctly reorders state updates before UI notifications.

    Medium
    General
    Prevent state-UI synchronization issues

    Move state updates before UI feedback to prevent potential race conditions where
    the UI updates before the state is fully synchronized.

    apps/platform/src/components/ui/combobox.tsx [75-80]

     if (success && data) {
    -  toast.success('Workspace created successfully')
       setSelectedWorkspace({ ...data, projects: 0 })
       setAllWorkspaces((prev) => [...prev, { ...data, projects: 0 }])
       setOpen(false)
    +  toast.success('Workspace created successfully')
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    __

    Why: While reordering state updates before the toast notification is a good practice to ensure UI consistency, the actual impact of potential race conditions in this case is minimal since React state updates are batched.

    Low
    • More

    @@ -157,7 +158,7 @@ export function Combobox(): React.JSX.Element {
    <AddSVG /> New workspace
    </Button>
    </DialogTrigger>
    <DialogContent>
    <DialogContent className="bg-[#1E1E1F]">
    Copy link
    Member

    Choose a reason for hiding this comment

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

    can you remove the transparency?

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    PLATFORM: Newly created workspace doesn't show up in list
    2 participants