-
Notifications
You must be signed in to change notification settings - Fork 2.3k
chore: placement menu leftpanel and ux create add projects #6636
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
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.
Caution
Changes requested ❌
Reviewed everything up to b4df56e in 1 minute and 32 seconds. Click for details.
- Reviewed
165lines of code in4files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/ThreadList.tsx:328
- Draft comment:
Avoid using the array index as the key for SortableItem; use thread.id instead to ensure stable identification during reordering. - 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.
2. web-app/src/routes/project/index.tsx:63
- Draft comment:
Use a route constant (e.g. route.project) instead of hardcoding the string '/project/$projectId' for navigation. This change promotes maintainability and consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment appears to be correct in principle - using route constants is generally better than hardcoded strings. However, looking at the Route definition (L24), it's defined as '/project/' without the $projectId parameter. I'm not certain if Route.project would actually work as a direct replacement since the route definition doesn't match the usage pattern. Without seeing more of the routing setup, I can't be 100% sure this suggestion is correct. The route constant defined in the file doesn't include the $projectId parameter, so the suggested change might not work. We also don't know if there are other route constants defined elsewhere that would be more appropriate. While using route constants is a good practice, we need to be certain that the suggested constant actually matches the required route pattern with parameters. Given the uncertainty about the routing setup and the mismatch between the defined Route constant and the required route pattern, we should not keep this comment.
Workflow ID: wflow_by9DDQrVuthWBfTV
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
LGTM
Describe Your Changes
Improved project creation and navigation UX with enhanced visual consistency across the
application.
Key Changes:
panel or project page, users are now automatically navigated to the newly created project
project view to match the main view theme
visual hierarchy
Technical Details
addFolder()
navigate to new projects automatically
project context
Test Plan
This improves the user experience by eliminating the extra step of manually navigating to
newly created projects while maintaining visual consistency across different views.
Fixes Issues
Self Checklist
Important
Enhances project creation UX with auto-navigation to new projects and improved visual consistency in menus and thread styling.
LeftPanel.tsx:219andproject/index.tsx:63.ThreadList.tsx:188for project context.addFolder()inuseThreadManagement.tsnow returns the new folder object.LeftPanel.tsxfor better visual hierarchy.This description was created by
for b4df56e. You can customize this summary. It will automatically update as commits are pushed.