-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[WEB-3203] fix: dashboard widgets' empty state content and assets #6450
Conversation
WalkthroughThis pull request encompasses a series of UI and styling modifications across multiple components in the web application. The changes primarily focus on refining the visual design, renaming components, updating text content, and improving the overall user interface consistency. Key areas of modification include checkbox styling in the Changes
Suggested labels
Suggested reviewers
Poem
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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: 0
🧹 Nitpick comments (8)
web/core/components/editor/sticky-editor/editor.tsx (1)
15-15
: LGTM! Consider updating component documentation.The renaming from
Toolbar
toStickyEditorToolbar
improves component specificity and clarity. The implementation remains functionally unchanged.Consider adding JSDoc comments to document the component's specific use case within the sticky editor context.
Also applies to: 90-102
web/core/components/home/widgets/empty-states/links.tsx (1)
5-6
: Consider standardizing size utility classes.The component uses both
size-[30px]
andsize-6
classes. Consider standardizing to use Tailwind's built-in size utilities consistently.- <div className="flex-shrink-0 size-[30px] grid place-items-center"> - <Link2 className="size-6 -rotate-45" /> + <div className="flex-shrink-0 size-8 grid place-items-center"> + <Link2 className="size-6 -rotate-45" />web/core/components/home/widgets/empty-states/stickies.tsx (1)
6-7
: Maintain consistent size utility classes across components.Similar to the links component, consider standardizing the size utilities.
- <div className="flex-shrink-0 size-[30px] grid place-items-center"> - <RecentStickyIcon className="size-6" /> + <div className="flex-shrink-0 size-8 grid place-items-center"> + <RecentStickyIcon className="size-6" />web/core/components/home/widgets/empty-states/recents.tsx (2)
5-28
: Consider using TypeScript enum for type parameter.The
type
parameter could benefit from stronger type safety using an enum or union type instead of a string.type RecentType = 'project' | 'page' | 'issue'; const getDisplayContent = (type: RecentType) => { // ... rest of the function };
35-36
: Standardize size utility classes for consistency.Maintain consistency with size utilities across all empty state components.
- <div className="flex-shrink-0 size-[30px] grid place-items-center"> - <displayContent.icon className="size-6" /> + <div className="flex-shrink-0 size-8 grid place-items-center"> + <displayContent.icon className="size-6" />web/core/components/home/widgets/links/link-detail.tsx (2)
Line range hint
41-42
: Add error handling for clipboard operations.The
handleCopyText
function should handle potential clipboard API failures.const handleCopyText = () => - copyTextToClipboard(viewLink).then(() => { + copyTextToClipboard(viewLink) + .then(() => { setToast({ type: TOAST_TYPE.SUCCESS, title: "Link Copied!", message: "View link copied to clipboard.", }); + }) + .catch((error) => { + setToast({ + type: TOAST_TYPE.ERROR, + title: "Failed to Copy", + message: "Could not copy link to clipboard.", + }); });
80-83
: Great UI improvements with room for enhancement.The transition effects and hover interactions are well implemented. Consider adding focus states for better keyboard accessibility.
- className="cursor-pointer group flex items-center bg-custom-background-100 px-4 w-[230px] h-[56px] border-[0.5px] border-custom-border-200 rounded-md gap-4 hover:shadow-md transition-shadow" + className="cursor-pointer group flex items-center bg-custom-background-100 px-4 w-[230px] h-[56px] border-[0.5px] border-custom-border-200 rounded-md gap-4 hover:shadow-md focus:shadow-md focus:outline-none focus:ring-2 focus:ring-custom-primary-200 transition-all"web/core/components/home/widgets/links/links.tsx (1)
Line range hint
36-45
: Consider performance optimization for large lists.The ContentOverflowWrapper with flex-wrap might cause performance issues with a large number of links.
Consider implementing virtualization for better performance with large datasets:
import { VirtualizedList } from '@plane/ui'; // Inside the component return ( <div className="relative"> <VirtualizedList data={links} height={150} itemSize={56} renderItem={({ index, style }) => ( <div style={style}> <ProjectLinkDetail key={links[index]} linkId={links[index]} linkOperations={linkOperations} /> </div> )} /> </div> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/editor/src/styles/editor.css
(1 hunks)web/core/components/editor/sticky-editor/editor.tsx
(2 hunks)web/core/components/editor/sticky-editor/toolbar.tsx
(4 hunks)web/core/components/home/home-dashboard-widgets.tsx
(1 hunks)web/core/components/home/widgets/empty-states/index.ts
(1 hunks)web/core/components/home/widgets/empty-states/links.tsx
(1 hunks)web/core/components/home/widgets/empty-states/no-projects.tsx
(4 hunks)web/core/components/home/widgets/empty-states/recents.tsx
(1 hunks)web/core/components/home/widgets/empty-states/stickies.tsx
(1 hunks)web/core/components/home/widgets/links/action.tsx
(1 hunks)web/core/components/home/widgets/links/create-update-link-modal.tsx
(2 hunks)web/core/components/home/widgets/links/link-detail.tsx
(2 hunks)web/core/components/home/widgets/links/links.tsx
(1 hunks)web/core/components/home/widgets/links/root.tsx
(1 hunks)web/core/components/home/widgets/recents/index.tsx
(4 hunks)web/core/components/stickies/layout/stickies-list.tsx
(1 hunks)web/core/constants/empty-state.tsx
(2 hunks)web/core/services/workspace.service.ts
(3 hunks)
✅ Files skipped from review due to trivial changes (4)
- web/core/components/home/widgets/links/action.tsx
- web/core/components/home/widgets/links/root.tsx
- web/core/components/home/home-dashboard-widgets.tsx
- web/core/components/home/widgets/links/create-update-link-modal.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (23)
web/core/components/editor/sticky-editor/toolbar.tsx (3)
26-26
: LGTM! Component name reflects its specific purpose.The rename from
Toolbar
toStickyEditorToolbar
improves code clarity by explicitly indicating its relationship with the sticky editor.
72-76
: Good accessibility improvement with button types.Adding
type="button"
to buttons prevents accidental form submissions and follows HTML best practices.Also applies to: 129-131
102-102
: Consistent styling update.The color class change from
text-custom-text-300
totext-custom-text-100/50
aligns with the design system's color hierarchy.packages/editor/src/styles/editor.css (1)
108-108
: Improved visual consistency in checkbox styling.The changes enhance the checkbox styling by:
- Using text color with opacity for borders, creating better visual harmony
- Implementing a consistent hover effect that aligns with the text color scheme
Also applies to: 115-115
web/core/services/workspace.service.ts (3)
19-19
: LGTM!The addition of
TActivityEntityData
type import follows TypeScript naming conventions and is properly placed with other type imports.
333-333
: LGTM!The addition of the return type annotation
Promise<TActivityEntityData[]>
improves type safety and maintains consistency with the method's implementation.
286-286
: Consider updating API endpoints for full consistency.While the comment has been updated from "quick links" to "quicklinks" for consistency, the API endpoints below still use "quick-links" with a hyphen. Consider updating the endpoints as well to maintain full consistency across the codebase.
Let's check the usage of "quick-links" vs "quicklinks" across the codebase:
web/core/components/home/widgets/empty-states/index.ts (1)
1-4
: LGTM! Clean export structure.The export statements are well-organized, following a modular approach by separating different empty state components into their respective modules.
web/core/components/home/widgets/empty-states/stickies.tsx (1)
9-11
: LGTM! Clear and engaging empty state message.The text content effectively communicates the purpose and encourages user action.
web/core/components/home/widgets/empty-states/recents.tsx (1)
31-39
: LGTM! Well-structured component with clear content.The component is well-organized with the separate
getDisplayContent
function and maintains consistent styling with other empty states. The messages are clear and informative for each type.web/core/components/home/widgets/recents/index.tsx (3)
14-14
: LGTM! Import changes improve component naming clarity.The renamed imports (
NoProjectsEmptyState
,RecentsEmptyState
) better reflect their specific purposes.
68-69
: LGTM! Empty state handling is more descriptive.The empty state component change from
EmptyWorkspace
toNoProjectsEmptyState
provides better context to users.
99-100
: LGTM! Added null safety with optional chaining.The addition of optional chaining (
?.
) before the filter method prevents potential runtime errors whenrecents
is null or undefined.web/core/components/home/widgets/empty-states/no-projects.tsx (3)
31-32
: LGTM! Text content improvements enhance clarity.The updated text content is more engaging and action-oriented:
- "Create a project." is more direct
- "Get started" is more inviting than the previous CTA
Also applies to: 35-35
33-33
: LGTM! Consistent icon sizing improves UI.Icons now use the standardized
size-12
class, improving visual consistency across the interface.Also applies to: 49-49, 59-59
70-76
: LGTM! Avatar component implementation is more robust.The Avatar component now includes proper props for better customization and accessibility:
- Proper source handling with fallback
- Name prop for accessibility
- Size specification
- Tooltip control
web/core/components/stickies/layout/stickies-list.tsx (1)
87-87
: LGTM! Improved empty state container spacing.Added horizontal margin (
mx-2
) improves the visual presentation while maintaining the grid layout and centering.web/core/constants/empty-state.tsx (2)
925-925
: LGTM! Enhanced stickies description is more engaging.The updated description better explains the purpose and value of stickies, making it more relatable to users' needs.
948-949
: LGTM! Home widgets empty state message is more positive.The new message frames the empty state more positively, encouraging users to re-enable widgets rather than focusing on their absence.
web/core/components/home/widgets/links/link-detail.tsx (3)
5-14
: LGTM! Clean import organization.The imports are well-organized into logical groups with clear comments.
Line range hint
21-32
: LGTM! Well-structured component implementation.Good use of observer pattern and proper null checks for linkDetail.
90-119
: Improve menu positioning and z-index handling.The current z-index implementation might cause issues with menu visibility in complex layouts.
Consider the following improvements:
- Use a higher z-index value to ensure menu visibility
- Add overflow handling for viewport edges
- Test menu positioning in different viewport sizes
- <CustomMenu placement="bottom-end" menuItemsClassName="z-20" closeOnSelect verticalEllipsis> + <CustomMenu + placement="bottom-end" + menuItemsClassName="z-[100]" + closeOnSelect + verticalEllipsis + containerClassName="relative" + >web/core/components/home/widgets/links/links.tsx (1)
41-43
: Consider edge cases in links rendering.While the simplified rendering logic is cleaner, ensure that
links
is always an array to prevent runtime errors.
Description
This PR updates-
Type of Change
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Style
Refactor