fix: new sticky color + recent sticky api call + sticky max height #6438
Conversation
WalkthroughThis pull request introduces modifications to the sticky note functionality across multiple files. The changes primarily focus on enhancing the flexibility of sticky data handling, introducing a random color generation for new sticky notes, and improving the Changes
Sequence DiagramsequenceDiagram
participant User
participant StickyComponent
participant StickyService
participant StickyStore
User->>StickyComponent: Create/Retrieve Sticky
StickyComponent->>StickyComponent: Generate Random Color
StickyComponent->>StickyService: Request Stickies
StickyService-->>StickyStore: Fetch Stickies
StickyStore-->>StickyService: Return Stickies
StickyService-->>StickyComponent: Return Stickies
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 (
|
![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
🧹 Nitpick comments (1)
web/core/services/sticky.service.ts (1)
24-25: LGTM! Consider documenting the pagination parameter.The addition of the optional
per_pageparameter enhances the flexibility of the API. The implementation correctly handles the default value using theSTICKIES_PER_PAGEconstant.Consider adding JSDoc comments to document the new parameter:
+ /** + * Fetches stickies with pagination support + * @param workspaceSlug - The workspace identifier + * @param cursor - The pagination cursor + * @param query - Optional search query + * @param per_page - Optional number of items per page (defaults to STICKIES_PER_PAGE) + */Also applies to: 30-30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/core/components/stickies/sticky/inputs.tsx(1 hunks)web/core/components/stickies/sticky/root.tsx(4 hunks)web/core/services/sticky.service.ts(1 hunks)web/core/store/sticky/sticky.store.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
web/core/components/stickies/sticky/inputs.tsx (1)
13-13: LGTM! Type change improves flexibility.Changing
stickyDatatoPartial<TSticky>is a good improvement as it:
- Better represents the reality that new stickies may not have all properties
- Aligns with the partial update pattern used in the codebase
- Reduces type coercion needs
web/core/components/stickies/sticky/root.tsx (2)
36-36: LGTM! Improved initialization of new stickies.The addition of random background color for new stickies improves the visual distinction between notes.
48-49: LGTM! Proper state handling in handleChange.The spread operator usage ensures all existing properties are preserved while allowing updates.
web/core/store/sticky/sticky.store.ts (1)
99-99: LGTM! Optimized recent sticky fetch.The modification to fetch only one sticky is more efficient than fetching the default page size when only the most recent sticky is needed.
| /> | ||
| <div | ||
| className={cn("w-full flex flex-col h-fit rounded p-4 group/sticky", className)} | ||
| className={cn("w-full flex flex-col h-fit rounded p-4 group/sticky max-h-[650px] overflow-y-scroll", className)} |
There was a problem hiding this comment.
💡 Codebase verification
Add ARIA attributes to improve accessibility of scrollable container
The scrollbar is correctly implemented with overflow-y-scroll, but the container needs additional accessibility attributes:
- Add
role="region"to identify it as a distinct section - Add
aria-labelto describe the scrollable content - Consider adding
tabIndex={0}if the container needs keyboard focus
Example:
className={cn(
"w-full flex flex-col h-fit rounded p-4 group/sticky max-h-[650px] overflow-y-scroll",
className
)}
role="region"
aria-label="Sticky note content"
tabIndex={0}🔗 Analysis chain
Verify max height constraint for accessibility.
While adding a max height constraint prevents infinite growth, consider:
- Ensuring the scrollbar is visible and usable
- Testing with screen readers
- Verifying keyboard navigation within scrollable content
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other max-height constraints in the codebase for consistency
rg "max-h-\[.*?\]" --type ts --type tsx
Length of output: 69
Script:
#!/bin/bash
# Search for max-height constraints with correct file types
rg "max-h-\[" -t typescript -t javascriptreact -t typescriptreact
# Search for scroll-related classes and ARIA attributes
rg "overflow-y-scroll|overflow-auto|aria-scrollable|tabIndex" -t typescript -t javascriptreact -t typescriptreact
# Search for accessibility-related configurations or utilities
fd -e ts -e tsx -e js -e jsx --exec grep -l "accessibility\|a11y"
Length of output: 354
Script:
#!/bin/bash
# Search for max-height and scroll-related classes
rg "max-h-|overflow-y-scroll|overflow-auto"
# Check content of the accessibility-related file
cat ./packages/ui/src/emoji/icons.ts
# Search for specific accessibility attributes
rg "aria-|role=|tabIndex"
Length of output: 82205
Description