fix: ensure empty state group header is visible #6606
Conversation
WalkthroughThis pull request adds an Changes
Sequence Diagram(s)sequenceDiagram
participant UI as GroupItem Component
participant Store as StateStore
participant TS as Translation Service
UI->>Store: Request grouped project states
Store-->>UI: Return grouped states (including empty arrays)
UI->>UI: Evaluate if empty state should render (using shouldShowEmptyState)
alt Empty State Needed
UI->>TS: Fetch localized empty state text
TS-->>UI: Return translated title and description
UI->>UI: Render empty state message
else
UI->>UI: Render existing states
end
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/i18n/src/locales/en/translations.json(1 hunks)packages/i18n/src/locales/es/translations.json(1 hunks)packages/i18n/src/locales/fr/translations.json(1 hunks)packages/i18n/src/locales/ja/translations.json(1 hunks)packages/i18n/src/locales/zh-CN/translations.json(1 hunks)web/core/components/project-states/group-item.tsx(3 hunks)web/core/store/state.store.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/store/state.store.ts
[error] 117-117: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (9)
web/core/components/project-states/group-item.tsx (3)
6-8: Well-organized imports with clear separation!Good practice using comments to separate imports into logical groups.
50-50: Good implementation of the empty state condition!The
shouldShowEmptyStatevariable effectively combines multiple conditions to determine when to show the empty state:
states.length === 0: No states existcurrentStateExpanded: Group is expanded!createState: Not currently creating a new state
86-91: Well-implemented empty state UI with proper i18n support!The empty state UI is well-structured:
- Uses proper conditional rendering
- Implements localization for better user experience
- Shows additional description for users with edit permissions
web/core/store/state.store.ts (1)
111-123: Good implementation ensuring all state groups are present!The logic correctly ensures that all state groups defined in STATE_GROUPS are represented in the returned object, even if they don't have any states. This aligns well with the PR's objective of fixing empty state visibility.
🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/i18n/src/locales/zh-CN/translations.json (1)
1691-1694: Well-structured translations for empty state!The Chinese translations are clear and maintain consistency with the application's messaging:
- Title properly includes the dynamic
{groupKey}parameter- Description provides a clear call-to-action
packages/i18n/src/locales/ja/translations.json (1)
1690-1694: LGTM! The empty state messages are well-structured and properly localized.The added empty state messages effectively handle the case when a state group has no states, providing clear feedback to users in Japanese. The use of the
{groupKey}placeholder allows for dynamic group names, making the messages reusable across different state groups.packages/i18n/src/locales/en/translations.json (1)
1521-1525:Details
✅ Verification successful
LGTM! The empty state messages are clear and well-structured.
The implementation follows i18n best practices with clear messages and proper dynamic content handling using the
{groupKey}parameter.Let's verify that the same structure exists in other language files:
🏁 Script executed:
#!/bin/bash # Description: Check if the empty state structure is consistent across all language files. # Search for empty_state structure in all language files rg -A 3 '"empty_state":\s*{' packages/i18n/src/locales/Length of output: 157
Below is a revised verification step using a simpler search pattern. The original regex caused an error, so please re-run the following shell script manually to verify that the
"empty_state"structure is consistently defined across all language files:Once you confirm that the structure (with its
titleanddescription) appears in the expected files, please update the review status accordingly.Next Action:
• Manually verify the output of this script to ensure consistency of the empty state structure in all i18n locale files.
🏁 Script executed:
[run_scripts] #!/bin/bash # Re-run search for the "empty_state" key (and the following three lines) to verify consistency rg -A 3 'empty_state' packages/i18n/src/locales/Length of output: 54995
LGTM – the empty state translation changes for the states group look solid.
• The English translation now presents a clear and concise message with the dynamic{groupKey}placeholder as intended ("No states available for the {groupKey} group", "Please create a new state").
• A manual review across the locale files (ES, FR, JA, ZH-CN) confirms that similar structures with appropriate localized placeholders and descriptions have been implemented.packages/i18n/src/locales/es/translations.json (1)
1690-1694: LGTM! Well-structured translations for empty states.The translations are well-organized and follow good practices:
- Proper nesting under the correct section
- Clear and natural Spanish translations
- Consistent placeholder usage for dynamic content
- Maintains semantic meaning from English to Spanish
packages/i18n/src/locales/fr/translations.json (1)
1690-1694: LGTM! The French translation for empty state is accurate and well-formatted.The added translation strings are properly formatted in French and maintain consistency with other language files. The
{groupKey}placeholder is correctly preserved for dynamic content.
| const allGroups = Object.keys(STATE_GROUPS).reduce( | ||
| (acc, group) => ({ | ||
| ...acc, | ||
| [group]: groupedStates[group] || [], | ||
| }), | ||
| {} as Record<string, IState[]> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize the reduce operation to avoid O(n²) complexity.
The spread operator in the reduce accumulator can lead to quadratic time complexity as it creates a new object and copies all properties in each iteration.
Consider this more efficient approach:
- const allGroups = Object.keys(STATE_GROUPS).reduce(
- (acc, group) => ({
- ...acc,
- [group]: groupedStates[group] || [],
- }),
- {} as Record<string, IState[]>
- );
+ const allGroups = {} as Record<string, IState[]>;
+ for (const group of Object.keys(STATE_GROUPS)) {
+ allGroups[group] = groupedStates[group] || [];
+ }📝 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.
| const allGroups = Object.keys(STATE_GROUPS).reduce( | |
| (acc, group) => ({ | |
| ...acc, | |
| [group]: groupedStates[group] || [], | |
| }), | |
| {} as Record<string, IState[]> | |
| ); | |
| const allGroups = {} as Record<string, IState[]>; | |
| for (const group of Object.keys(STATE_GROUPS)) { | |
| allGroups[group] = groupedStates[group] || []; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
Description
Fixes an issue where the empty state group header was not appearing in settings when all states were deleted for a group. Ensures it renders correctly even when no items are present.
Type of Change
Screenshots and Media (if applicable)
Summary by CodeRabbit