[WEB-2226]chore: updated 'Issue states' settings ui #6338
Conversation
WalkthroughThis pull request introduces modifications to several components within the project states management system. The changes primarily focus on enhancing the user interface and interaction for state management, including improvements to form inputs, group item rendering, and state item styling. The modifications include adding a TextArea for descriptions, implementing group expansion functionality, and refining the visual presentation of state-related components. Changes
Sequence DiagramsequenceDiagram
participant User
participant GroupList
participant GroupItem
participant StateItem
User->>GroupList: Interact with states
GroupList->>GroupList: Manage groupsExpanded state
GroupList->>GroupItem: Pass expansion handlers
GroupItem->>GroupItem: Toggle group visibility
GroupItem->>StateItem: Render states when expanded
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
web/core/components/project-states/create-update/form.tsx (1)
Line range hint
31-46: Improve error handling and user feedback.The current error handling only logs to console, which isn't helpful for users. Consider implementing proper error feedback.
const formSubmit = async (event: FormEvent<HTMLFormElement>) => { event.preventDefault(); const name = formData?.name || undefined; if (!formData || !name) { let currentErrors: Partial<Record<keyof IState, string>> = {}; if (!name) currentErrors = { ...currentErrors, name: "Name is required" }; setErrors(currentErrors); return; } try { await onSubmit(formData); } catch (error) { - console.log("error", error); + // Set form-level error + setErrors((prev) => ({ + ...prev, + form: "Failed to save state. Please try again.", + })); + // Optionally, propagate error to parent + props.onError?.(error); } };You'll need to:
- Add
onErrorto theTStateFormtype- Display the form-level error message to users
🧹 Nitpick comments (6)
web/core/components/project-states/create-update/form.tsx (2)
86-94: Consider adjusting TextArea layout behavior.While the TextArea implementation is good, consider setting both min-height and max-height to prevent unexpected layout shifts during user input.
- className="w-full text-sm min-h-14 resize-none" + className="w-full text-sm min-h-14 max-h-32 resize-y"This change would:
- Allow vertical resizing for better UX
- Prevent the form from growing too tall
- Maintain a consistent layout
96-103: Add loading state to the submit button.Consider adding a loading indicator to provide visual feedback during form submission.
- <Button type="submit" variant="primary" size="sm" disabled={buttonDisabled}> + <Button + type="submit" + variant="primary" + size="sm" + disabled={buttonDisabled} + loading={isSubmitting} + > {buttonTitle} </Button>You'll need to add an
isSubmittingstate:const [isSubmitting, setIsSubmitting] = useState(false); const formSubmit = async (event: FormEvent<HTMLFormElement>) => { event.preventDefault(); setIsSubmitting(true); try { await onSubmit(formData); } catch (error) { console.log("error", error); } finally { setIsSubmitting(false); } };web/core/components/project-states/group-list.tsx (2)
20-27: Consider memoizing the collapse handler.While the implementation is correct, consider using useCallback for the handler to prevent unnecessary re-renders of child components.
- const handleGroupCollapse = (groupKey: TStateGroups) => { + const handleGroupCollapse = useCallback((groupKey: TStateGroups) => { setGroupsExpanded((prev) => { if (prev.includes(groupKey)) { return prev.filter((key) => key !== groupKey); } return prev; }); - }; + }, []);
29-36: Consider memoizing the expand handler.Similar to the collapse handler, consider using useCallback here as well.
- const handleExpand = (groupKey: TStateGroups) => { + const handleExpand = useCallback((groupKey: TStateGroups) => { setGroupsExpanded((prev) => { if (prev.includes(groupKey)) { return prev; } return [...prev, groupKey]; }); - }; + }, []);web/core/components/project-states/group-item.tsx (1)
42-45: Consider memoizing derived values.The
currentStateExpandedvalue could be memoized to prevent unnecessary recalculations.+ const currentStateExpanded = useMemo(() => + groupsExpanded.includes(groupKey), + [groupsExpanded, groupKey] + ); - const currentStateExpanded = groupsExpanded.includes(groupKey);web/core/components/project-states/state-item.tsx (1)
131-131: Consider extracting className to a constant.The className string is quite long and could be extracted for better maintainability.
+ const stateItemClassName = cn( + "relative border border-custom-border-100 bg-custom-background-100 py-3 px-3.5 rounded group", + isDragging ? `opacity-50` : `opacity-100`, + totalStates === 1 ? `cursor-auto` : `cursor-grab` + ); <div ref={draggableElementRef} - className={cn( - "relative border border-custom-border-100 bg-custom-background-100 py-3 px-3.5 rounded group", - isDragging ? `opacity-50` : `opacity-100`, - totalStates === 1 ? `cursor-auto` : `cursor-grab` - )} + className={stateItemClassName} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/core/components/project-states/create-update/form.tsx(2 hunks)web/core/components/project-states/group-item.tsx(1 hunks)web/core/components/project-states/group-list.tsx(3 hunks)web/core/components/project-states/state-item-title.tsx(1 hunks)web/core/components/project-states/state-item.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-web
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
web/core/components/project-states/create-update/form.tsx (2)
6-6: LGTM! Import changes are well-organized.The addition of TextArea import aligns with the form's enhanced description field functionality.
62-70: Well-structured layout with proper spacing!The form layout effectively uses flexbox for:
- Horizontal spacing between color picker and inputs
- Proper containment of the color picker
- Consistent vertical spacing in the main content area
web/core/components/project-states/group-list.tsx (1)
17-18: LGTM! Well-typed state management.The state initialization with proper TypeScript types ensures type safety.
web/core/components/project-states/state-item-title.tsx (1)
26-26: Verify padding removal impact.The removal of padding classes (
py-4 px-2) might affect the component's layout and touch targets for interactive elements.✅ Verification successful
Padding removal is properly handled.
The removal of padding classes is safe as spacing is maintained through:
- Flex container with gap utilities
- Inner text container padding (px-2)
- Properly sized interactive elements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if padding is handled by parent components rg -l 'py-[2-4]|px-[2-4]' --type ts --type tsxLength of output: 76
Script:
#!/bin/bash # Search for padding classes in TypeScript and TypeScript React files rg -l 'py-[2-4]|px-[2-4]' --type-add 'tsx:*.{tsx,ts}' --type tsx # Get context around StateItemTitle usage rg -B 3 -A 3 "StateItemTitle" --type-add 'tsx:*.{tsx,ts}' --type tsx # Look at the complete file content to understand component structure cat web/core/components/project-states/state-item-title.tsxLength of output: 30631
web/core/components/project-states/group-item.tsx (1)
19-21: LGTM! Well-defined prop types.The new props are properly typed with TypeScript, ensuring type safety.
web/core/components/project-states/state-item.tsx (1)
110-110: LGTM! Proper effect dependencies.The addition of
disabledto the dependency array is correct as it affects the drag behavior.
| <div className="flex justify-between items-center gap-2"> | ||
| <div | ||
| className="w-full flex items-center cursor-pointer py-1" | ||
| onClick={() => (!currentStateExpanded ? handleExpand(groupKey) : handleGroupCollapse(groupKey))} | ||
| > | ||
| <div | ||
| className="flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden transition-colors hover:bg-custom-background-80 cursor-pointer text-custom-primary-100/80 hover:text-custom-primary-100" | ||
| onClick={() => !createState && setCreateState(true)} | ||
| className={cn( | ||
| "flex-shrink-0 w-5 h-5 rounded flex justify-center items-center overflow-hidden transition-all", | ||
| { | ||
| "rotate-0": currentStateExpanded, | ||
| "-rotate-90": !currentStateExpanded, | ||
| } | ||
| )} | ||
| > | ||
| <Plus className="w-4 h-4" /> | ||
| <ChevronDown className="w-4 h-4" /> | ||
| </div> | ||
| <div className="flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden"> | ||
| <StateGroupIcon stateGroup={groupKey} height="16px" width="16px" /> | ||
| </div> | ||
| )} | ||
| <div className="text-base font-medium text-custom-text-200 capitalize px-1">{groupKey}</div> | ||
| </div> | ||
| <div | ||
| className="flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden transition-colors hover:bg-custom-background-80 cursor-pointer text-custom-primary-100/80 hover:text-custom-primary-100" | ||
| onClick={() => !createState && setCreateState(true)} | ||
| > | ||
| <Plus className="w-4 h-4" /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add ARIA attributes for accessibility.
The expandable section should have proper ARIA attributes for better accessibility.
<div
className="flex justify-between items-center gap-2"
+ role="button"
+ aria-expanded={currentStateExpanded}
+ aria-controls={`group-${groupKey}-content`}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="flex justify-between items-center gap-2"> | |
| <div | |
| className="w-full flex items-center cursor-pointer py-1" | |
| onClick={() => (!currentStateExpanded ? handleExpand(groupKey) : handleGroupCollapse(groupKey))} | |
| > | |
| <div | |
| className="flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden transition-colors hover:bg-custom-background-80 cursor-pointer text-custom-primary-100/80 hover:text-custom-primary-100" | |
| onClick={() => !createState && setCreateState(true)} | |
| className={cn( | |
| "flex-shrink-0 w-5 h-5 rounded flex justify-center items-center overflow-hidden transition-all", | |
| { | |
| "rotate-0": currentStateExpanded, | |
| "-rotate-90": !currentStateExpanded, | |
| } | |
| )} | |
| > | |
| <Plus className="w-4 h-4" /> | |
| <ChevronDown className="w-4 h-4" /> | |
| </div> | |
| <div className="flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden"> | |
| <StateGroupIcon stateGroup={groupKey} height="16px" width="16px" /> | |
| </div> | |
| )} | |
| <div className="text-base font-medium text-custom-text-200 capitalize px-1">{groupKey}</div> | |
| </div> | |
| <div | |
| className="flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden transition-colors hover:bg-custom-background-80 cursor-pointer text-custom-primary-100/80 hover:text-custom-primary-100" | |
| onClick={() => !createState && setCreateState(true)} | |
| > | |
| <Plus className="w-4 h-4" /> | |
| </div> | |
| <div | |
| className="flex justify-between items-center gap-2" | |
| role="button" | |
| aria-expanded={currentStateExpanded} | |
| aria-controls={`group-${groupKey}-content`} | |
| > | |
| <div | |
| className="w-full flex items-center cursor-pointer py-1" | |
| onClick={() => (!currentStateExpanded ? handleExpand(groupKey) : handleGroupCollapse(groupKey))} | |
| > | |
| <div | |
| className={cn( | |
| "flex-shrink-0 w-5 h-5 rounded flex justify-center items-center overflow-hidden transition-all", | |
| { | |
| "rotate-0": currentStateExpanded, | |
| "-rotate-90": !currentStateExpanded, | |
| } | |
| )} | |
| > | |
| <ChevronDown className="w-4 h-4" /> | |
| </div> | |
| <div className="flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden"> | |
| <StateGroupIcon stateGroup={groupKey} height="16px" width="16px" /> | |
| </div> | |
| <div className="text-base font-medium text-custom-text-200 capitalize px-1">{groupKey}</div> | |
| </div> | |
| <div | |
| className="flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden transition-colors hover:bg-custom-background-80 cursor-pointer text-custom-primary-100/80 hover:text-custom-primary-100" | |
| onClick={() => !createState && setCreateState(true)} | |
| > | |
| <Plus className="w-4 h-4" /> | |
| </div> |
Description
Updates 'Issue states' settings to match with project states settings.
Type of Change
References
WEB-2226
Summary by CodeRabbit
New Features
Bug Fixes
Style