-
Notifications
You must be signed in to change notification settings - Fork 690
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: add message to display list being empty #300
Conversation
Warning Rate limit exceeded@amhsirak has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThe pull request adds a new localization feature across multiple language files (German, English, Spanish, Japanese, and Chinese) by introducing a "list empty" message in the Changes
Possibly related PRs
Poem
🪧 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locales/de.json
(1 hunks)public/locales/en.json
(1 hunks)public/locales/es.json
(1 hunks)public/locales/ja.json
(1 hunks)public/locales/zh.json
(1 hunks)src/components/organisms/RightSidePanel.tsx
(1 hunks)
🔇 Additional comments (7)
src/components/organisms/RightSidePanel.tsx (1)
548-550
: Consider adding a null/undefined check for step.fields
.
If there's any scenario where step.fields
might be undefined or null, Object.entries(step.fields)
will throw an error. If you're certain step.fields
is always defined at this point, you can ignore this. Otherwise, consider optional chaining or a defensive check.
public/locales/zh.json (1)
194-195
: Translation addition looks good.
These new keys accurately convey an empty list message in a concise manner, consistent with existing translations.
public/locales/ja.json (1)
194-195
: New “list_empty” message is well-formed.
The addition is grammatically correct and maintains consistency with the structure of other keys.
public/locales/en.json (1)
194-195
: Localization string addition is appropriate.
The new message aligns with existing wording, improving clarity when the list contains no fields.
public/locales/es.json (1)
194-195
: LGTM! Spanish translation is accurate and well-formatted.
The translation "La lista está vacía. No se han seleccionado campos." is grammatically correct and effectively conveys the empty list state to Spanish-speaking users.
public/locales/de.json (2)
193-194
: LGTM! German translation is accurate and well-formatted.
The translation "Liste ist leer. Es wurden keine Felder ausgewählt." is grammatically correct and effectively conveys the empty list state to German-speaking users.
193-195
: Verify translations in other language files.
The AI summary mentions translations in English, Japanese, and Chinese files. Let's verify they exist and are consistent.
✅ Verification successful
Translations are consistent across all language files
The verification shows that the "list_empty" message exists in all language files (en, ja, zh, de) with appropriate translations:
- English: "List is empty. No fields have been selected."
- Japanese: "リストが空です。フィールドが選択されていません。"
- Chinese: "列表为空。尚未选择任何字段。"
- German: "Liste ist leer. Es wurden keine Felder ausgewählt."
Each translation maintains the same meaning while being culturally appropriate for its target language.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the "list_empty" message exists in all language files
# and display their translations for comparison.
echo "Checking translations in all language files..."
for lang in en ja zh; do
if [ -f "public/locales/${lang}.json" ]; then
echo "=== ${lang}.json ==="
# Extract the list_empty message using jq
jq -r '.right_panel.messages.list_empty' "public/locales/${lang}.json"
else
echo "Warning: public/locales/${lang}.json not found"
fi
done
Length of output: 679
<> | ||
<Typography>{t('right_panel.messages.list_selected')}</Typography> | ||
{Object.entries(step.fields).map(([key, field]) => ( | ||
<Box key={key}> | ||
<TextField | ||
label={t('right_panel.fields.field_label')} | ||
value={field.label || ''} | ||
onChange={(e) => handleTextLabelChange(field.id, e.target.value, step.id, key)} | ||
fullWidth | ||
margin="normal" | ||
InputProps={{ | ||
readOnly: confirmedListTextFields[field.id]?.[key], | ||
startAdornment: ( | ||
<InputAdornment position="start"> | ||
<EditIcon /> | ||
</InputAdornment> | ||
) | ||
}} | ||
/> | ||
<TextField | ||
label={t('right_panel.fields.field_data')} | ||
value={field.data || ''} | ||
fullWidth | ||
margin="normal" | ||
InputProps={{ | ||
readOnly: true, | ||
startAdornment: ( | ||
<InputAdornment position="start"> | ||
<TextFieldsIcon /> | ||
</InputAdornment> | ||
) | ||
}} | ||
/> | ||
{!confirmedListTextFields[step.id]?.[key] && ( | ||
<Box display="flex" justifyContent="space-between" gap={2}> | ||
<Button | ||
variant="contained" | ||
onClick={() => handleListTextFieldConfirm(step.id, key)} | ||
disabled={!field.label?.trim()} | ||
> | ||
{t('right_panel.buttons.confirm')} | ||
</Button> | ||
<Button | ||
variant="contained" | ||
color="error" | ||
onClick={() => handleListTextFieldDiscard(step.id, key)} | ||
> | ||
{t('right_panel.buttons.discard')} | ||
</Button> | ||
</Box> | ||
)} | ||
</Box> | ||
))} | ||
</> |
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.
Fix mismatched indices in readOnly
condition for list fields.
Currently, the code uses confirmedListTextFields[field.id]?.[key]
to determine if the field is read-only. However, the confirm/discard logic uses handleListTextFieldConfirm(step.id, key)
and handleListTextFieldDiscard(step.id, key)
, storing confirmation state in confirmedListTextFields[step.id][key]
. This mismatch prevents the field from toggling read-only properly.
Below is a proposed fix for the relevant read-only line, switching to step.id
so it matches the stored structure:
- readOnly: confirmedListTextFields[field.id]?.[key],
+ readOnly: confirmedListTextFields[step.id]?.[key],
📝 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.
<> | |
<Typography>{t('right_panel.messages.list_selected')}</Typography> | |
{Object.entries(step.fields).map(([key, field]) => ( | |
<Box key={key}> | |
<TextField | |
label={t('right_panel.fields.field_label')} | |
value={field.label || ''} | |
onChange={(e) => handleTextLabelChange(field.id, e.target.value, step.id, key)} | |
fullWidth | |
margin="normal" | |
InputProps={{ | |
readOnly: confirmedListTextFields[field.id]?.[key], | |
startAdornment: ( | |
<InputAdornment position="start"> | |
<EditIcon /> | |
</InputAdornment> | |
) | |
}} | |
/> | |
<TextField | |
label={t('right_panel.fields.field_data')} | |
value={field.data || ''} | |
fullWidth | |
margin="normal" | |
InputProps={{ | |
readOnly: true, | |
startAdornment: ( | |
<InputAdornment position="start"> | |
<TextFieldsIcon /> | |
</InputAdornment> | |
) | |
}} | |
/> | |
{!confirmedListTextFields[step.id]?.[key] && ( | |
<Box display="flex" justifyContent="space-between" gap={2}> | |
<Button | |
variant="contained" | |
onClick={() => handleListTextFieldConfirm(step.id, key)} | |
disabled={!field.label?.trim()} | |
> | |
{t('right_panel.buttons.confirm')} | |
</Button> | |
<Button | |
variant="contained" | |
color="error" | |
onClick={() => handleListTextFieldDiscard(step.id, key)} | |
> | |
{t('right_panel.buttons.discard')} | |
</Button> | |
</Box> | |
)} | |
</Box> | |
))} | |
</> | |
<> | |
<Typography>{t('right_panel.messages.list_selected')}</Typography> | |
{Object.entries(step.fields).map(([key, field]) => ( | |
<Box key={key}> | |
<TextField | |
label={t('right_panel.fields.field_label')} | |
value={field.label || ''} | |
onChange={(e) => handleTextLabelChange(field.id, e.target.value, step.id, key)} | |
fullWidth | |
margin="normal" | |
InputProps={{ | |
readOnly: confirmedListTextFields[step.id]?.[key], | |
startAdornment: ( | |
<InputAdornment position="start"> | |
<EditIcon /> | |
</InputAdornment> | |
) | |
}} | |
/> | |
<TextField | |
label={t('right_panel.fields.field_data')} | |
value={field.data || ''} | |
fullWidth | |
margin="normal" | |
InputProps={{ | |
readOnly: true, | |
startAdornment: ( | |
<InputAdornment position="start"> | |
<TextFieldsIcon /> | |
</InputAdornment> | |
) | |
}} | |
/> | |
{!confirmedListTextFields[step.id]?.[key] && ( | |
<Box display="flex" justifyContent="space-between" gap={2}> | |
<Button | |
variant="contained" | |
onClick={() => handleListTextFieldConfirm(step.id, key)} | |
disabled={!field.label?.trim()} | |
> | |
{t('right_panel.buttons.confirm')} | |
</Button> | |
<Button | |
variant="contained" | |
color="error" | |
onClick={() => handleListTextFieldDiscard(step.id, key)} | |
> | |
{t('right_panel.buttons.discard')} | |
</Button> | |
</Box> | |
)} | |
</Box> | |
))} | |
</> |
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.
@RohitR311 resolve merge conflicts
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)
src/components/recorder/RightSidePanel.tsx (1)
750-752
: LGTM! Consider enhancing the empty state UI.The empty state message implementation is clean and well-integrated with the translation system.
Consider enhancing the empty state with a visual indicator (like an icon) and possibly a call-to-action to guide users on how to add fields to the list. Example:
-<Typography>{t('right_panel.messages.list_empty')}</Typography> +<Box display="flex" flexDirection="column" alignItems="center" gap={2}> + <ListIcon sx={{ fontSize: 48, color: 'text.secondary' }} /> + <Typography>{t('right_panel.messages.list_empty')}</Typography> + <Typography variant="body2" color="text.secondary"> + {t('right_panel.messages.list_empty_help')} + </Typography> +</Box>
If in Capture List action all data fields are discarded display a message in the right side panel stating no list items.
Summary by CodeRabbit
New Features
User Experience