-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[WEB-3078] chore: empty state config #6397
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-3078] chore: empty state config #6397
Conversation
WalkthroughThis pull request updates the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial 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 (1)
web/core/constants/empty-state.ts (1)
654-663: LGTM! Configurations are well-structured.The new epic relation configurations follow the established pattern and maintain consistency with existing entries.
Consider adding a brief comment above these entries to document when these empty states are displayed, similar to how other sections are documented (e.g., "// Teams", "// project pages").
+ // Epic relations [EmptyStateType.EPIC_RELATION_SEARCH_EMPTY_STATE]: { key: EmptyStateType.EPIC_RELATION_SEARCH_EMPTY_STATE, title: "No matching epics found", path: "/empty-state/search/search", },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/constants/empty-state.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
web/core/constants/empty-state.ts (2)
645-645: LGTM! Text correction improves accuracy.The title text has been corrected, maintaining consistency with other similar messages.
86-87: LGTM! New enum entries follow conventions.The new epic relation enum entries are well-structured and properly placed near other relation-related entries.
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 (2)
web/core/components/dropdowns/date-range.tsx (2)
53-53: Add JSDoc documentation for the new prop.Consider adding JSDoc documentation to explain the purpose and behavior of the
renderPlaceholderprop. This will help other developers understand when and how to use this prop.+ /** + * Controls whether placeholder text should be rendered when dates are not selected. + * @default true + */ renderPlaceholder?: boolean;
172-172: Consider refactoring for better readability and DRY principle.The placeholder rendering logic is duplicated and uses nested ternaries. Consider extracting this into a helper function for better readability and maintainability.
+ const renderDateOrPlaceholder = (date: Date | undefined, placeholder: string) => + date ? renderFormattedDate(date) : (renderPlaceholder ? placeholder : ""); + // Then use it in JSX: - {dateRange.from ? renderFormattedDate(dateRange.from) : renderPlaceholder ? placeholder.from : ""} + {renderDateOrPlaceholder(dateRange.from, placeholder.from)} - {dateRange.to ? renderFormattedDate(dateRange.to) : renderPlaceholder ? placeholder.to : ""} + {renderDateOrPlaceholder(dateRange.to, placeholder.to)}Also applies to: 179-179
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/dropdowns/date-range.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
web/core/components/dropdowns/date-range.tsx (1)
86-86: LGTM!The default value of
truemaintains backward compatibility with existing usage of the component.
Description
This PR includes following changes:
Type of Change
References
[WEB-3078]
Summary by CodeRabbit
New Features
DateRangeDropdowncomponent with an optional property to control placeholder rendering.Bug Fixes
Style