-
Notifications
You must be signed in to change notification settings - Fork 32
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: dynamic project tags UI and color generation system #136 #170
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughThis pull request implements new tag management functionality in the web application. A new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant PT as ProjectTags Component
participant TM as TagManager Component
participant HT as useTags Hook
participant UT as Utility Functions
U->>PT: Interact with tag UI
PT->>TM: Render TagManager
U->>TM: Submit new tag or request edit/remove
TM->>HT: Invoke tag management (add/edit/remove)
HT->>UT: Format tag name & get random color
UT-->>HT: Return processed tag data
HT-->>TM: Update tag list
TM->>U: Refresh UI with updated tags
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (8)
apps/web/app/(routes)/project/create/page.tsx (1)
6-14
: Consider passing tag management functions to TagManager.The component correctly uses the
useTags
hook, but the retrieved functions (addTag
,removeTag
,updateTag
) aren't being passed to theTagManager
component. This could indicate thatTagManager
is internally using the same hook, which might lead to unnecessary hook duplications.- <TagManager /> + <TagManager + tags={tags} + onAdd={addTag} + onRemove={removeTag} + onUpdate={updateTag} + />apps/web/components/shared/tag-edit.tsx (2)
19-19
: Consider implementing the commented error state.The commented error state suggests a planned feature. Implementing it would provide better user feedback beyond just toast notifications.
-// const [error, setError] = useState('') +const [error, setError] = useState('')
21-36
: Enhance form validation with proper error states.The form validation could be more robust by setting error states and providing immediate feedback.
const handleSubmit = useCallback( (e: React.FormEvent) => { e.preventDefault() - if (editedTag.trim()) { + const trimmedTag = editedTag.trim() + setError('') + + if (!trimmedTag) { + setError('Tag name cannot be empty') + toast.error('Tag name cannot be empty') + return + } + + const formattedNewTag = formatToPascalCase(trimmedTag) + if (formattedNewTag !== tag.text) { onUpdate(tag.id, formattedNewTag) } else { onCancel() } - } else { - toast.error('Tag name cannot be empty') - } }, - [editedTag, tag, onUpdate, onCancel], + [editedTag, tag, onUpdate, onCancel, setError], )apps/web/lib/utils/tag-context.ts (2)
33-42
: Consider making color selection deterministic for consistent tag colors.Currently, colors are randomly assigned. Making this deterministic based on the tag name would ensure consistent colors across sessions.
-export function getRandomAccessibleColor(): { +export function getTagColor(tagName: string): { backgroundColor: string textColor: string } { - const randomIndex = Math.floor(Math.random() * ACCESSIBLE_COLORS.length) + // Use hash of tag name to determine color index + const hash = tagName.split('').reduce((acc, char) => { + return char.charCodeAt(0) + ((acc << 5) - acc) + }, 0) + const index = Math.abs(hash) % ACCESSIBLE_COLORS.length return { - backgroundColor: ACCESSIBLE_COLORS[randomIndex].bg, - textColor: ACCESSIBLE_COLORS[randomIndex].text, + backgroundColor: ACCESSIBLE_COLORS[index].bg, + textColor: ACCESSIBLE_COLORS[index].text, } }
47-63
: Optimize tag addition with proper memoization.The
addTag
callback hastags
in its dependency array, which could cause unnecessary rerenders.const addTag = useCallback( (tagName: string) => { const formattedTag = formatToPascalCase(tagName) - if (!tags.some((tag) => tag.text === formattedTag)) { - const { backgroundColor, textColor } = getRandomAccessibleColor() - setTags((prevTags) => [ + setTags((prevTags) => { + if (prevTags.some((tag) => tag.text === formattedTag)) { + return prevTags + } + const { backgroundColor, textColor } = getTagColor(formattedTag) + return [ ...prevTags, { id: crypto.randomUUID(), text: formattedTag, color: { backgroundColor, textColor }, }, - ]) - } + ] + }) }, - [tags], + [], )apps/web/lib/types/project.types.ts (1)
35-38
: Consider creating a dedicated type for tag colors.The color object structure could be extracted into a reusable type, especially since it's used across multiple components.
+/** Interface representing tag color configuration */ +export interface TagColor { + /** Background color in hex format */ + backgroundColor: string + /** Text color in hex format */ + textColor: string +} export interface ProjectTag { /** Unique identifier for the tag */ id: string /** Display text for the tag */ text: string - color: { - backgroundColor: string - textColor: string - } + /** Color configuration for the tag */ + color: TagColor }apps/web/components/shared/tag-manager.tsx (2)
12-17
: Consider enhancing error handling implementationThe commented out error state suggests a potential enhancement opportunity. While toast notifications provide immediate feedback, consider implementing persistent error state for more complex error scenarios, such as validation errors that need to remain visible while the user corrects their input.
- // const [error, setError] = useState('') + const [errors, setErrors] = useState<Record<string, string>>({})
54-108
: Enhance user experience with additional states and confirmationsConsider implementing these UX improvements:
- Add loading states during tag operations
- Handle empty state when no tags exist
- Add confirmation dialog for tag deletion to prevent accidental removals
+ {tags.length === 0 && ( + <p className="text-gray-500">No tags yet. Create your first tag above.</p> + )}For the delete confirmation:
- onClick={() => removeTag(tag.id)} + onClick={() => { + if (window.confirm(`Are you sure you want to delete "${tag.text}"?`)) { + removeTag(tag.id) + } + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(routes)/project/create/page.tsx
(1 hunks)apps/web/components/shared/tag-edit.tsx
(1 hunks)apps/web/components/shared/tag-manager.tsx
(1 hunks)apps/web/lib/types/project.types.ts
(1 hunks)apps/web/lib/utils/tag-context.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: Review TypeScript files ensuring type safety with...
**/*.ts
: Review TypeScript files ensuring type safety with no 'any' usage unless justified. Verify named exports, proper error handling with typed errors, and pure functions. Check module encapsulation and consistent naming using camelCase for functions and PascalCase for types. Validate unit test coverage for utilities if required.
apps/web/lib/types/project.types.ts
apps/web/lib/utils/tag-context.ts
🔇 Additional comments (1)
apps/web/components/shared/tag-manager.tsx (1)
1-11
: Well-structured imports and setup!The imports are well-organized and each one serves a clear purpose in the component.
Hey @ussyalfaks I see some build fails, can you please check? 💡 Always do |
Hey @ussyalfaks I noticed that the first commit in your PR is still without a signature. Can you please check that out? I will be sharing you a steps to follow to sign old commits: cc - @Bran18 |
@ussyalfaks did you check the suggestion and request from Andler? |
This PR implements the frontend components and logic for the dynamic project tags system, enabling users to create and manage tags with automatically generated colors.
Features Implemented
✅ Dynamic Color Generation for new tags
✅ Tag Normalization to PascalCase (supports separated words)
✅ Class Name Generation based on tag names
✅ Tag Management UI for creating, editing, and displaying tags
✅ State Management to handle tag data
✅ Consistent Tag Display with generated colors
✅ Error Handling and feedback for users
Technical Details
Tag Normalization: Converts tag names to PascalCase
Deterministic Color Generation: Ensures consistent colors for the same tag names
User Feedback: UI provides visual confirmation of tag creation, delete, edits, and updates
To view the implementation, navigate to:
📂 web/app/routes/project/create
Summary by CodeRabbit