-
Notifications
You must be signed in to change notification settings - Fork 47
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
fix(web): Check UI and fix language issue #1237
Conversation
✅ Deploy Preview for reearth-web ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe pull request introduces extensive modifications across various components and localization files, primarily enhancing internationalization support. Key changes include the addition of new translation keys, updates to existing strings for clarity, and the incorporation of localization functions in component titles and button labels. Notably, a new Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (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
Documentation and Community
|
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: 14
🧹 Outside diff range and nitpick comments (37)
web/src/beta/features/Editor/Widgets/WidgetManagerPanel/ListItem.tsx (1)
Line range hint
1-58
: Well-structured internationalization implementationThe component maintains its core functionality while properly implementing internationalization. The changes are minimal and focused, following React best practices for hooks and memoization.
Consider documenting the translation keys used in this component in a central location to help maintain consistency as the application grows.
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/LayerStyle/index.tsx (1)
Line range hint
33-42
: Consider adding error logging for invalid input typesThe current implementation silently returns when receiving invalid input types. Consider adding development-time logging to help with debugging.
const handleLayerStyleChange = useCallback( (value?: string | string[]) => { - if (typeof value !== "string") return; + if (typeof value !== "string") { + if (process.env.NODE_ENV === 'development') { + console.warn('LayerStyleTab: Expected string value but received:', value); + } + return; + } onLayerConfigUpdate?.({ layerId: selectedLayerId, config: {web/src/beta/features/Dashboard/ContentsContainer/Projects/ProjectRemoveModal.tsx (1)
Line range hint
34-38
: Consider adding aria-label for better accessibility.The translation implementation looks good. However, since this is a destructive action, consider enhancing accessibility by adding an aria-label that provides more context.
<Button size="normal" title={t("Remove")} + aria-label={t("Remove this project")} appearance="dangerous" disabled={disabled} onClick={onProjectRemove} />
web/src/beta/features/AssetsManager/AssetsSelector.tsx (1)
Line range hint
43-47
: Maintain consistency in translationsThe "Select" button title is still hardcoded while "Cancel" has been translated. For consistency in internationalization, the "Select" button's title should also use the translation function.
Apply this diff to fix the inconsistency:
<Button size="normal" - title="Select" + title={t("Select")} appearance="primary" onClick={handleSelectFinish} />web/src/beta/features/Editor/Map/SketchLayerEditor/index.tsx (1)
Line range hint
54-61
: Consider standardizing button order and appearanceFor better UX consistency, consider following these common modal button patterns:
- Place the primary action ("Apply") on the right
- Use consistent button sizes
- Consider adding a visual separator between buttons
actions={ <> - <Button onClick={handleClose} size="normal" title={t("Cancel")} /> - <Button - size="normal" - title={t("Apply")} - appearance="primary" - onClick={handleSubmit} - disabled={warning} - /> + <ButtonGroup> + <Button onClick={handleClose} size="medium" title={t("Cancel")} /> + <Button + size="medium" + title={t("Apply")} + appearance="primary" + onClick={handleSubmit} + disabled={warning} + /> + </ButtonGroup> </> }web/src/beta/features/WorkspaceSetting/innerPages/Workspaces/DeleteWorkspaceModal.tsx (4)
Line range hint
54-63
: LGTM! Consider applying consistent styling to both buttons.The translation implementation and safety checks look good. The
nowrap
styling on the cancel button is a good UX improvement.Consider wrapping both buttons in styled components for consistency:
+const ActionButton = styled(Button)(() => ({ + whiteSpace: "nowrap" +})); - <Button + <ActionButton key="delete" title={t("I am sure I want to delete this workspace.")} appearance="dangerous" disabled={workspaceNameConfirm.trim() !== workspace?.name?.trim()} onClick={handleDeleteWorkspace} />
84-84
: Consider using a more specific translation key.While the translation implementation is correct, the key "your workspace name" is quite generic. Consider using a more specific key like "deleteWorkspace.confirmationInput.placeholder" to better organize translations.
- placeholder={t("your workspace name")} + placeholder={t("deleteWorkspace.confirmationInput.placeholder")}
94-96
: LGTM! Consider adding explicit TypeScript type.The styled component implementation is clean and focused. For better type safety and documentation, consider adding an explicit type annotation.
-const CancelButton = styled(Button)(() => ({ +const CancelButton: typeof Button = styled(Button)(() => ({ whiteSpace: "nowrap" }));
Line range hint
32-39
: Consider enhancing error handling in delete operation.The workspace deletion handler should include error handling and user feedback for failed operations.
const handleDeleteWorkspace = useCallback(async () => { if (!workspace?.id) return; - await deleteWorkspace({ teamId: workspace.id }); - onClose(); - setWorkspace(undefined); - navigate(`/`); + try { + await deleteWorkspace({ teamId: workspace.id }); + onClose(); + setWorkspace(undefined); + navigate(`/`); + } catch (error) { + console.error('Failed to delete workspace:', error); + // TODO: Add user feedback for failed operation + } }, [workspace, deleteWorkspace, setWorkspace, navigate, onClose]);web/src/beta/utils/time.ts (2)
25-34
: Consider performance and maintainability improvements.A few suggestions to enhance the code:
- Move the
units
object outside the function to prevent recreation on each call- Consider using an enum or union type for unit keys
- Consider moving translations to a dedicated i18n configuration file
Here's a suggested improvement:
+const TIME_UNITS = { + year: { value: 31536000, label: "year", jaLabel: "年" }, + month: { value: 2592000, label: "month", jaLabel: "ヶ月" }, + day: { value: 86400, label: "day", jaLabel: "日" }, + hour: { value: 3600, label: "hour", jaLabel: "時間" }, + minute: { value: 60, label: "minute", jaLabel: "分" }, + second: { value: 1, label: "second", jaLabel: "秒" } +} as const; + +type TimeUnit = keyof typeof TIME_UNITS; + export const formatRelativeTime = (date: Date, lang = "en"): string => { const seconds = Math.floor((new Date().getTime() - date.getTime()) / 1000); - - const units: Record< - string, - { value: number; label: string; jaLabel: string } - > = { - year: { value: 31536000, label: "year", jaLabel: "年" }, - month: { value: 2592000, label: "month", jaLabel: "ヶ月" }, - day: { value: 86400, label: "day", jaLabel: "日" }, - hour: { value: 3600, label: "hour", jaLabel: "時間" }, - minute: { value: 60, label: "minute", jaLabel: "分" }, - second: { value: 1, label: "second", jaLabel: "秒" } - };
22-49
: Consider adopting a more robust i18n solution.While the current implementation works for basic English and Japanese support, consider using a dedicated i18n library (like i18next) for better maintainability and scalability. This would provide:
- Centralized translation management
- Better pluralization rules
- Support for more languages
- Runtime language switching
web/src/beta/ui/fields/TimePointField/EditPanel/index.tsx (1)
Line range hint
32-91
: Consider enhancing error handling and accessibilityWhile the current implementation works well, consider these improvements:
- Add error handling for DatePicker and TimePicker components
- Enhance accessibility by adding aria-labels to the timezone selector
Here's a suggested implementation:
<DatePicker value={date} onChange={handleDateChange} + onError={(error) => console.error('Date parsing error:', error)} /> <TimePicker value={time} onChange={handleTimeChange} + onError={(error) => console.error('Time parsing error:', error)} /> <Selector value={timezone} options={TIMEZONE_OFFSETS.map((offset) => ({ label: offset, value: offset }))} maxHeight={200} onChange={handleTimezoneSelect} + aria-label={t("Select timezone")} />web/src/beta/lib/reearth-ui/components/CodeInput/index.tsx (1)
2-2
: LGTM! Consider enhancing accessibility.The i18n integration is well-implemented. Consider adding aria-label for better accessibility:
- loading={<>{t("Loading...")}</>} + loading={<div aria-label={t("Loading...")}>{t("Loading...")}</div>}Also applies to: 26-26, 100-100
web/src/beta/ui/fields/ListField/ListItem.tsx (1)
Line range hint
52-56
: Consider translating the "Rename" menu itemFor consistency with the "Delete" menu item translation, consider translating the "Rename" text as well.
menu.unshift({ id: "rename", - title: "Rename", + title: t("Rename"), icon: "pencilSimple" as const, onClick: () => setItemNameRenameId(item.id) });web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx (2)
Line range hint
42-63
: Consider extracting schema construction logic.The
handleSubmit
function handles both schema construction and submission. Consider extracting the schema construction logic into a separate utility function for better maintainability and testability.+ const constructSchema = (properties: CustomPropertyProp[]) => + properties.reduce((acc, property, index) => { + const [key] = Object.keys(property); + const value = `${property[key]}_${index + 1}`; + return { ...acc, [key]: value }; + }, {}); const handleSubmit = () => { - const schemaJSON = customProperties.reduce((acc, property, index) => { - const [key] = Object.keys(property); - const value = `${property[key]}_${index + 1}`; - acc[key] = value; - return acc; - }, {}); + const schemaJSON = constructSchema(customProperties); onSubmit?.({ layerType: "simple", sceneId: sceneId || "",
Line range hint
31-36
: Consider renaming the warning state.The
warning
state could be more descriptive. Consider renaming it to better reflect its purpose, such ashasInvalidProperties
orcustomPropertiesError
.- const [warning, setWarning] = useState(false); + const [hasInvalidProperties, setHasInvalidProperties] = useState(false);web/src/beta/features/CreateWorkspaceModal/index.tsx (2)
78-78
: Consider a more descriptive placeholderThe current placeholder repeats the label text, which could be redundant. Consider using a more helpful example or hint instead.
- placeholder={t("your workspace name")} + placeholder={t("e.g., Marketing Team 2024")}
Line range hint
28-31
: Add input validation for workspace namesWhile duplicate name validation is well implemented, consider adding more comprehensive input validation.
const handleAddWorkspace = useCallback(async () => { + const name = workspaceNameConfirm.trim(); + if (name.length < 3 || name.length > 50) { + return; // Add error handling + } + if (!/^[\w\s-]+$/.test(name)) { + return; // Add error handling + } const { data } = await addWorkspace({ - name: workspaceNameConfirm + name }); setAddWorkspaceModal(false); navigate(`/dashboard/${data?.id}`); }, [addWorkspace, workspaceNameConfirm, setAddWorkspaceModal, navigate]);web/src/beta/lib/reearth-ui/components/Modal/index.stories.tsx (1)
Line range hint
16-93
: Consider adding accessibility attributes to the Modal storiesAs these stories serve as documentation and examples, consider enhancing them with proper accessibility attributes:
- Add
aria-label
to the modal- Include
role="dialog"
- Add
aria-describedby
for the modal contentThis would demonstrate best practices for accessibility in the component usage.
web/src/beta/features/Visualizer/Crust/StoryPanel/Page/hooks.ts (1)
83-88
: Maintain consistent i18n usageWhile "Title" and "Color" are properly internationalized, the "Padding" string at line 95 remains untranslated. Consider applying the same i18n pattern for consistency.
padding: { value: property?.title?.padding, - title: "Padding", + title: t("Padding"), type: "spacing", ui: "padding" }Also applies to: 95-95
web/src/beta/ui/fields/ListField/index.tsx (2)
119-119
: Consider using a more specific translation keyWhile the translation works, consider using a more specific, namespaced key to avoid potential conflicts and provide better context for translators.
- title={t("New Item")} + title={t("listField.addNewItem")}This change would:
- Prevent key collisions with other "New Item" translations
- Provide clear context for translators about where this string appears
- Make it easier to maintain translations as the application grows
Internationalization needs improvement in ListItem component
The verification revealed that the "Rename" text in ListItem's options menu is not internationalized. Additionally, there's a hardcoded string comparison
item.title === "New Credit"
.Required changes:
- Internationalize the "Rename" text in
optionsMenu
usingt("Rename")
- Replace the hardcoded "New Credit" string comparison with a proper i18n key
🔗 Analysis chain
Line range hint
1-144
: Verify complete internationalization coverageWhile the "New Item" button has been internationalized, please verify if there are any other user-facing strings that need translation, particularly:
- Error messages
- ARIA labels for accessibility
- Dynamic content in the ListItem component
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential hardcoded strings in the component and related files echo "Searching for potential hardcoded strings..." rg -g '*.tsx' -g '*.ts' --type-add 'web:*.{tsx,ts}' -t web 'aria-label=["'\'']|title=["'\'']|placeholder=["'\'']|error=["'\'']' ./web/src/beta/ui/fields/ # Check ListItem component for any text that needs translation echo "Checking ListItem component..." cat ./web/src/beta/ui/fields/ListField/ListItem.tsxLength of output: 5286
web/src/beta/features/Editor/Story/PagesPanel/PageItem.tsx (1)
76-77
: Consider localizing the "Rename" option for consistencyFor consistency with the localized "Delete" option, consider updating the "Rename" string to use the translation function as well.
{ id: "rename", - title: "Rename", + title: t("Rename"), icon: "pencilSimple" as const, onClick: () => setEditingPageNameId(storyPage.id) },web/src/beta/ui/fields/Properties/ProprtyField.tsx (1)
183-183
: Consider translating remaining hardcoded stringsWhile "min" and "max" are now properly translated, there are other hardcoded strings in the component that could benefit from translation for consistency, such as:
- "Button radio field"
- "URL"
- The fallback text "{schema.name} field"
web/src/beta/features/Editor/Map/LayerStylePanel/Editor/StyleInterface/StyleNode/index.tsx (1)
59-65
: Consider translating additional user-facing stringsWhile the "Delete" string is correctly translated, there are other user-facing strings in the component that could benefit from translation for consistency:
- The node title displayed in the TitleWrapper
- The valueOptions labels used in the dropdown
Consider applying translations to these strings:
<TitleWrapper> - {node.title} + {t(node.title)} </TitleWrapper> // In the valueOptions useMemo valueOptions?.map((v) => ({ value: v, - label: v + label: t(v) }))Note: If these strings are already translated upstream or if they are dynamic values that shouldn't be translated, please disregard this suggestion.
web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx (2)
62-68
: Consider centralizing translation keysWhile the translations are correctly implemented, consider extracting the translation keys into a centralized constants file to improve maintainability and reduce the risk of typos.
Example implementation:
// constants/translations.ts export const LAYER_MENU = { RENAME: 'Rename', DELETE: 'Delete', PROPERTY_SCHEMA: 'Property Schema', } as const; // LayerItem.tsx import { LAYER_MENU } from 'constants/translations'; // ... title: t(LAYER_MENU.RENAME)Also applies to: 78-78
Line range hint
174-176
: Consider improving title editing accessibilityThe double-click interaction for editing the title might not be discoverable. Consider adding a tooltip and appropriate ARIA attributes to improve accessibility.
Example implementation:
<TitleWrapper onDoubleClick={() => setEditingLayerNameId(layer.id)} + title={t("Double click to edit")} + role="button" + aria-label={t("Double click to edit layer name")} > {localTitle} </TitleWrapper>web/src/beta/features/Dashboard/ContentsContainer/Projects/Project/ProjectListViewItem.tsx (1)
Line range hint
108-108
: Update Typography components to use new variable namesAfter renaming the date variables, update their usage in the Typography components:
-<Typography size="body">{UpdatedAt}</Typography> +<Typography size="body">{updatedAt}</Typography> ... -<Typography size="body">{createAt}</Typography> +<Typography size="body">{createdAt}</Typography>Also applies to: 111-111
web/src/beta/lib/reearth-ui/components/Selector/index.tsx (2)
131-131
: Consider using a translation key constantWhile the internationalization implementation is correct, consider extracting "Please select" into a translation key constant to maintain consistency across the codebase and ease maintenance.
- {placeholder ? placeholder : t("Please select")} + {placeholder ? placeholder : t(TRANSLATION_KEYS.PLEASE_SELECT)}Add to your translation keys file:
export const TRANSLATION_KEYS = { PLEASE_SELECT: "Please select", // ... other keys } as const;
40-40
: Consider adding error handling for translationsWhile the translation implementation is functional, consider wrapping it in an error boundary or try-catch block to gracefully handle missing translations or i18n service failures.
const getPlaceholder = (customPlaceholder?: string) => { try { return customPlaceholder ?? t(TRANSLATION_KEYS.PLEASE_SELECT); } catch (error) { console.error('Translation failed:', error); return customPlaceholder ?? "Please select"; // Fallback to English } };Then use it in your render:
- {placeholder ? placeholder : t("Please select")} + {getPlaceholder(placeholder)}Also applies to: 131-131
web/src/beta/features/AssetsManager/hooks.ts (1)
238-241
: Consider implementing folder support and path click handlerThe TODO comment and empty path click handler suggest incomplete folder navigation functionality:
- The TODO indicates pending folder support
- The
handlePathClick
handler is defined but not implementedThis could impact the user's ability to navigate through asset folders effectively.
Would you like me to help implement the folder navigation functionality or create a GitHub issue to track this task?
web/src/services/i18n/translations/en.yml (2)
67-67
: Consider using more specific terminology for destructive actionsThe term "Remove" might be ambiguous. Consider using more specific terms based on the context (e.g., "Delete", "Remove Layer", etc.) to make the consequences clearer to users.
392-395
: Consider using full words instead of abbreviationsThe terms "min" and "max" are abbreviated. Consider using full words "Minimum" and "Maximum" for better clarity and accessibility.
-min: '' -max: '' +Minimum: '' +Maximum: ''web/src/services/i18n/translations/ja.yml (3)
39-40
: Consider enhancing workspace name validation messages.While the workspace name translations are good, consider adding specific validation messages for:
- Maximum length restrictions
- Allowed characters
- Reserved names
This would help users understand the requirements before encountering validation errors.
395-398
: Consider adding tooltips or help text for new UI elements.While the translations for new items, credits, min, and max are accurate, consider adding:
- Tooltip translations to explain their purpose
- Help text translations for first-time users
- Context-specific usage examples
Line range hint
1-500
: Maintain consistent punctuation in system messages.There are inconsistencies in how periods (.) are used in system messages. Consider establishing and following consistent rules for:
- Ending punctuation in error messages
- Punctuation in success messages
- Usage of spaces after periods
server/pkg/builtin/manifest_ja.yml (1)
1422-1422
: Fix YAML formatting issuesPlease address the following YAML formatting issues:
- Add a newline at the end of file
- Remove trailing spaces on line 1422
Apply this diff:
- +🧰 Tools
🪛 yamllint
[error] 1422-1422: no new line character at the end of file
(new-line-at-end-of-file)
[error] 1422-1422: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (33)
server/pkg/builtin/manifest_ja.yml
(3 hunks)web/src/beta/features/AssetsManager/AssetsSelector.tsx
(1 hunks)web/src/beta/features/AssetsManager/hooks.ts
(1 hunks)web/src/beta/features/CreateWorkspaceModal/index.tsx
(1 hunks)web/src/beta/features/Dashboard/ContentsContainer/Projects/Project/ProjectListViewItem.tsx
(2 hunks)web/src/beta/features/Dashboard/ContentsContainer/Projects/ProjectRemoveModal.tsx
(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/LayerStyle/index.tsx
(1 hunks)web/src/beta/features/Editor/Map/LayerStylePanel/Editor/StyleInterface/StyleNode/index.tsx
(2 hunks)web/src/beta/features/Editor/Map/LayerStylePanel/LayerStyleItem.tsx
(3 hunks)web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx
(5 hunks)web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx
(1 hunks)web/src/beta/features/Editor/Map/SketchLayerEditor/index.tsx
(1 hunks)web/src/beta/features/Editor/Story/PagesPanel/PageItem.tsx
(1 hunks)web/src/beta/features/Editor/Story/PagesPanel/index.tsx
(1 hunks)web/src/beta/features/Editor/Widgets/WidgetManagerPanel/ListItem.tsx
(2 hunks)web/src/beta/features/Visualizer/Crust/StoryPanel/Page/hooks.ts
(4 hunks)web/src/beta/features/WorkspaceSetting/innerPages/Members/index.tsx
(2 hunks)web/src/beta/features/WorkspaceSetting/innerPages/Workspaces/DeleteWorkspaceModal.tsx
(4 hunks)web/src/beta/lib/reearth-ui/components/CodeInput/index.tsx
(3 hunks)web/src/beta/lib/reearth-ui/components/ColorInput/index.tsx
(3 hunks)web/src/beta/lib/reearth-ui/components/Modal/index.stories.tsx
(3 hunks)web/src/beta/lib/reearth-ui/components/Selector/index.tsx
(3 hunks)web/src/beta/ui/fields/CameraField/CapturePanel.tsx
(1 hunks)web/src/beta/ui/fields/CameraField/EditorPanel.tsx
(1 hunks)web/src/beta/ui/fields/ListField/ListItem.tsx
(4 hunks)web/src/beta/ui/fields/ListField/index.tsx
(3 hunks)web/src/beta/ui/fields/Properties/ProprtyField.tsx
(3 hunks)web/src/beta/ui/fields/TimePeriodField/EditPanel/index.tsx
(1 hunks)web/src/beta/ui/fields/TimePeriodField/index.tsx
(1 hunks)web/src/beta/ui/fields/TimePointField/EditPanel/index.tsx
(1 hunks)web/src/beta/utils/time.ts
(1 hunks)web/src/services/i18n/translations/en.yml
(10 hunks)web/src/services/i18n/translations/ja.yml
(13 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/src/beta/ui/fields/TimePeriodField/index.tsx
🧰 Additional context used
🪛 yamllint
server/pkg/builtin/manifest_ja.yml
[error] 1422-1422: no new line character at the end of file
(new-line-at-end-of-file)
[error] 1422-1422: trailing spaces
(trailing-spaces)
🪛 Biome
web/src/beta/lib/reearth-ui/components/CodeInput/index.tsx
[error] 100-100: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (50)
web/src/beta/features/Editor/Widgets/WidgetManagerPanel/ListItem.tsx (4)
3-3
: LGTM: Clean import addition
The useT
hook import is properly placed and follows the project's import structure.
25-25
: Verify translation key consistency
Let's ensure the "Delete" translation key is consistent with similar actions across the application.
#!/bin/bash
# Search for other occurrences of Delete translation
rg "t\(\"Delete\"\)" --type ts --type tsx
# Search for similar action translations
ast-grep --pattern 'title: t($_)'
20-20
: Verify complete translation coverage
While the "Delete" string is now translated, let's ensure we haven't missed any other translatable strings in the component.
✅ Verification successful
All strings are properly handled - no untranslated text found
After reviewing the complete component, all text content is either:
- Dynamically passed through props (
item.title
) - Already using the translation hook (
t("Delete")
) - Or consists of only styled components with no hardcoded user-facing strings
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential hardcoded strings in the component
rg -U "title=\{(?!\<)|\"[A-Z][a-zA-Z ]+\"" web/src/beta/features/Editor/Widgets/WidgetManagerPanel/ListItem.tsx
Length of output: 355
Script:
#!/bin/bash
# Search for potential hardcoded strings in the component using a simpler pattern
rg -U "title=\"|title=\{|>[\w\s]+<" web/src/beta/features/Editor/Widgets/WidgetManagerPanel/ListItem.tsx
Length of output: 218
Script:
#!/bin/bash
# Search for string literals in the component
rg -U "'[^']*'|\"[^\"]*\"" web/src/beta/features/Editor/Widgets/WidgetManagerPanel/ListItem.tsx
Length of output: 584
Script:
#!/bin/bash
# Get the full content of the file for context
cat web/src/beta/features/Editor/Widgets/WidgetManagerPanel/ListItem.tsx
Length of output: 1396
30-30
: LGTM: Proper dependency array update
The t
function is correctly added to the dependency array. This ensures the options menu updates when the translation function changes.
✅ Verification successful
Based on the search results, I can see that the pattern of using the t
function in useMemo
dependencies is consistently followed across the codebase. The search revealed numerous examples where translation function t
is properly included in the dependency array when used within the memoized value, which aligns with the change made in the review.
LGTM: Consistent pattern across codebase
The addition of t
to the dependency array follows the established pattern in the codebase, where translation function is consistently included in useMemo
dependencies when used within the callback.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar patterns to ensure consistency
ast-grep --pattern 'useMemo($_, [$$$, t])'
Length of output: 43903
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/LayerStyle/index.tsx (2)
26-26
: LGTM: Proper internationalization of the "NO STYLE" label
The translation implementation is correct and follows the established pattern using the useT
hook.
29-29
: LGTM: Correct dependency array update
The addition of t
to the dependency array is essential to ensure translations update properly when the language changes.
web/src/beta/features/Dashboard/ContentsContainer/Projects/ProjectRemoveModal.tsx (1)
31-31
: LGTM! Cancel button translation looks good.
The translation implementation is correct and consistent with the internationalization strategy.
web/src/beta/features/AssetsManager/AssetsSelector.tsx (1)
42-42
: LGTM! Translation function properly implemented.
The Cancel button's title is now correctly using the translation function.
web/src/beta/ui/fields/CameraField/CapturePanel.tsx (1)
28-34
: LGTM! Verify translation keys exist.
The internationalization changes for the button titles look good. The implementation follows React best practices and maintains consistent styling.
Let's verify that the required translation keys exist:
web/src/beta/features/Editor/Map/LayerStylePanel/LayerStyleItem.tsx (2)
3-3
: LGTM: Clean i18n integration
The addition of the useT
hook and its initialization follows React best practices and maintains consistent code organization.
Also applies to: 27-27
69-69
: Verify other hardcoded strings in the component
While the "Delete" string has been properly internationalized, there might be other user-facing strings in the component that should also be translated.
✅ Verification successful
No other hardcoded strings found in the component
After reviewing the component's source code, all user-facing strings are properly handled. The only text string present is "Delete" which is already correctly internationalized using the t()
function. Other text content is either dynamic (from props/state) or styling-related values that don't require translation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential hardcoded strings in the component that might need translation
rg -U --type tsx 'title[^=]*=\s*[^t]"|\'|`[^`]+`' web/src/beta/features/Editor/Map/LayerStylePanel/LayerStyleItem.tsx
Length of output: 93
Script:
#!/bin/bash
# Search for potential hardcoded strings in the component that might need translation
# Looking for string literals that could be titles, labels, or text content
rg -U --type tsx '"[^"]*"|\'[^\']*\'' web/src/beta/features/Editor/Map/LayerStylePanel/LayerStyleItem.tsx
Length of output: 93
Script:
#!/bin/bash
# Search for potential hardcoded strings in the component
cat web/src/beta/features/Editor/Map/LayerStylePanel/LayerStyleItem.tsx
Length of output: 2273
web/src/beta/features/Editor/Map/SketchLayerEditor/index.tsx (1)
Line range hint 54-61
: LGTM! Proper implementation of localization
The translation implementation for button titles is correct and aligns with the PR objectives to fix language issues. The useT
hook is properly utilized for both "Cancel" and "Apply" buttons.
web/src/beta/ui/fields/CameraField/EditorPanel.tsx (1)
46-52
: LGTM! Localization implementation looks good.
The translation implementation for button titles follows best practices using the useT
hook, and the button order (Cancel before Apply) follows common UI patterns.
Let's verify consistent translation key usage across the codebase:
✅ Verification successful
Translation implementation is consistent across the codebase
The verification shows that all interactive components consistently use t("Cancel")
and t("Apply")
for button translations throughout the application, with the only exceptions being in story files (which are for component documentation). The implementation in EditorPanel.tsx
follows the established pattern used across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of "Cancel" and "Apply" translation keys
# Expected: All occurrences should use t("Cancel") and t("Apply") for consistency
# Search for Cancel/Apply string usage
echo "Checking Cancel/Apply translation patterns:"
rg -U 'title=.*(?:Cancel|Apply)' --type tsx
# Search for direct t function calls with Cancel/Apply
echo "Checking t function usage with Cancel/Apply:"
rg -U 't\(["\'](?:Cancel|Apply)["\']' --type tsx
Length of output: 457
Script:
#!/bin/bash
# Let's try a different approach without file type restriction
# Search for Cancel/Apply translation patterns in all files
echo "Checking Cancel/Apply translation patterns:"
rg -U "title=.*['\"](?:Cancel|Apply)['\"]"
# Search for t function usage with Cancel/Apply
echo "Checking t function usage with Cancel/Apply:"
rg -U "t\\(['\"](?:Cancel|Apply)['\"]"
Length of output: 7214
web/src/beta/ui/fields/TimePeriodField/EditPanel/index.tsx (1)
Line range hint 44-51
: LGTM! Button localization and order changes look good.
The changes improve internationalization support and follow common UX patterns by:
- Using the
t
function for button titles - Placing the "Cancel" button before the "Apply" button, which is a standard modal button order
web/src/beta/utils/time.ts (1)
22-24
: LGTM! Backward compatible language support added.
The function signature change maintains backward compatibility while adding internationalization support.
web/src/beta/ui/fields/TimePointField/EditPanel/index.tsx (1)
44-50
: LGTM! Button localization looks good.
The localization of button titles using the useT
hook is implemented correctly. The button order (Cancel before Apply) follows common UI patterns, and the styling is consistent.
Let's verify the translation keys are defined:
✅ Verification successful
Translation keys "Cancel" and "Apply" are properly defined in the translation files
The translation keys are correctly defined in both English (web/src/services/i18n/translations/en.yml
) and Japanese (web/src/services/i18n/translations/ja.yml
) translation files:
- "Cancel": defined in both files (キャンセル in Japanese)
- "Apply": defined in both files (適用 in Japanese)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the translation keys "Cancel" and "Apply" are defined in localization files
# Expected: Find these keys in translation files to ensure they're available
# Search for the translation keys in all yml files
rg -l "Cancel:|Apply:" "**/*.yml"
Length of output: 84
Script:
#!/bin/bash
# Let's try a different approach to find translation files
# First, let's find all translation-related files
fd -e yaml -e yml
# Then, let's also check for translation keys in other potential formats (json, ts)
rg -l "Cancel.*:|Apply.*:" --type json --type ts
Length of output: 1178
Script:
#!/bin/bash
# Let's check the content of the English translation file for these keys
cat web/src/services/i18n/translations/en.yml
# Also check Japanese translation file as it seems to be supported
cat web/src/services/i18n/translations/ja.yml
Length of output: 33345
web/src/beta/features/Editor/Story/PagesPanel/index.tsx (2)
Line range hint 82-89
: LGTM! Verify translation key existence
The Button component's translation and accessibility implementation (icon + title) is correct. Let's verify the "New Page" translation key exists in all language files.
#!/bin/bash
# Description: Check if the "New Page" translation key exists in all language files
# Expect: The key should be present in both English and Japanese language files
# Search for "New Page" key in language files
echo "Checking language files for 'New Page' key..."
rg -l "New Page:" web/src/beta/lang/
73-73
: Verify translation key existence
The translation implementation is correct, but let's ensure the "Pages" key exists in all language files to prevent missing translations.
web/src/beta/ui/fields/ListField/ListItem.tsx (2)
3-3
: LGTM: Clean i18n hook implementation
The import and initialization of the translation hook follow React best practices.
Also applies to: 39-39
44-44
: LGTM: Proper translation of menu items
The menu text translation is correctly implemented, and the translation function is properly included in the useMemo dependencies.
Also applies to: 60-60
web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx (1)
Line range hint 121-128
: LGTM! Verify translation keys exist.
The internationalization changes for button titles are implemented correctly using the useT
hook.
Let's verify that the translation keys exist in the locale files:
✅ Verification successful
Translation keys are properly configured and used consistently
The translation keys "Cancel" and "Create" are properly defined in both English (en.yml) and Japanese (ja.yml) translation files, and they are used consistently across the codebase. The implementation in SketchLayerCreator matches the established pattern of translation key usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the "Cancel" and "Create" translation keys exist in locale files
# Expected: Both keys should be present in en.yml and ja.yml
# Search for the translation keys in locale files
echo "Checking for 'Cancel' translation key:"
rg -l "Cancel:" web/src/locales/
echo -e "\nChecking for 'Create' translation key:"
rg -l "Create:" web/src/locales/
Length of output: 371
Script:
#!/bin/bash
# Let's first find the locale files directory
fd -t f "en.yml|ja.yml|en.json|ja.json" -x echo "Found locale file: {}"
# Then let's try searching in the entire codebase for translation keys
echo -e "\nSearching for translation key usage patterns:"
rg "t\(['\"]Cancel['\"]" --type ts --type tsx
rg "t\(['\"]Create['\"]" --type ts --type tsx
# Let's also check if there's any i18n config or setup files
echo -e "\nSearching for i18n configuration:"
fd -t f "i18n" --exec echo "Found i18n related file: {}"
Length of output: 1036
Script:
#!/bin/bash
# Let's check the content of the translation files for these specific keys
echo "Checking English translations:"
cat "./web/src/services/i18n/translations/en.yml"
echo -e "\nChecking Japanese translations:"
cat "./web/src/services/i18n/translations/ja.yml"
# Let's also check the translation usage with proper file type
echo -e "\nChecking translation key usage:"
rg -t typescript -t tsx "t\(['\"]Cancel['\"]"
rg -t typescript -t tsx "t\(['\"]Create['\"]"
Length of output: 33752
Script:
#!/bin/bash
# Let's search for translation keys in the codebase with proper file extensions
echo "Checking translation key usage:"
rg "t\(['\"]Cancel['\"]" --type-add 'ts:*.{ts,tsx}' --type ts
rg "t\(['\"]Create['\"]" --type-add 'ts:*.{ts,tsx}' --type ts
# Let's also check if these keys are used in other forms
echo -e "\nChecking other forms of these translations:"
rg "Cancel:" --type yaml
rg "Create:" --type yaml
Length of output: 3211
web/src/beta/lib/reearth-ui/components/Modal/index.stories.tsx (1)
1-1
: LGTM: Clean i18n integration
The internationalization setup follows React best practices with proper hook placement and import structure.
Also applies to: 21-21
web/src/beta/features/Visualizer/Crust/StoryPanel/Page/hooks.ts (1)
2-2
: LGTM: Proper i18n hook integration
The useT
hook is correctly imported and initialized at the component level, following React hooks best practices.
Also applies to: 29-29
web/src/beta/ui/fields/ListField/index.tsx (1)
2-2
: LGTM: Clean integration of i18n support
The addition of the useT
hook and its initialization follows React best practices and is properly integrated with the existing imports.
Also applies to: 43-44
web/src/beta/features/Editor/Story/PagesPanel/PageItem.tsx (1)
78-78
: LGTM! Proper localization of the delete option
The changes correctly implement localization for the delete menu option and properly include the translation function in the dependency array.
Also applies to: 83-83
web/src/beta/ui/fields/Properties/ProprtyField.tsx (2)
4-4
: LGTM: Proper i18n hook implementation
The useT
hook is correctly imported and initialized at the component level, following React hooks best practices.
Also applies to: 42-43
Line range hint 1-191
: Fix typo in filename: "ProprtyField.tsx" → "PropertyField.tsx"
The filename contains a typo ("Proprty" instead of "Property"). This should be corrected to maintain consistency with the component name and improve code maintainability.
Let's verify the impact of this rename:
✅ Verification successful
Let me check the actual imports to verify the impact.
Fix typo in filename: "ProprtyField.tsx" → "PropertyField.tsx"
The file can be safely renamed as it's only imported from the local index.tsx file using a relative import. The component is correctly imported as PropertyField
, so the rename will only require updating one import statement in index.tsx
.
Locations to update:
- Rename
web/src/beta/ui/fields/Properties/ProprtyField.tsx
toPropertyField.tsx
- Update import in
web/src/beta/ui/fields/Properties/index.tsx
from"./ProprtyField"
to"./PropertyField"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for files importing this component
rg -l "ProprtyField"
# Check if the component is imported with a different path alias
rg -l "PropertyField"
Length of output: 1609
Script:
#!/bin/bash
# Check the actual import statement in the index file
rg -A 2 "ProprtyField" "web/src/beta/ui/fields/Properties/index.tsx"
# Check if there are any other direct imports of this file
rg "from.*ProprtyField"
Length of output: 240
web/src/beta/features/Editor/Map/LayerStylePanel/Editor/StyleInterface/StyleNode/index.tsx (2)
2-2
: LGTM: Import statement properly added
The useT
hook import is correctly added and aligns with the internationalization requirements.
54-55
: LGTM: Hook initialization follows best practices
The useT
hook is properly initialized at the component's top level, following React hooks rules.
web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx (3)
8-8
: LGTM: Import added for internationalization support
The useT
hook import is correctly added to support the internationalization changes.
38-38
: LGTM: Translation hook properly initialized
The useT
hook is correctly initialized at the component level, following React conventions.
95-96
: LGTM: Proper dependency management
The translation function is correctly added to the dependency array, ensuring proper reactivity when the language changes.
web/src/beta/features/WorkspaceSetting/innerPages/Members/index.tsx (1)
97-97
: LGTM! Good internationalization implementation.
The translation is correctly implemented using the translation mapping object.
web/src/beta/features/Dashboard/ContentsContainer/Projects/Project/ProjectListViewItem.tsx (1)
8-8
: LGTM: Clean hook integration for user preferences
The addition of useMeFetcher
and its implementation follows React best practices and properly integrates user preferences into the component.
Also applies to: 26-28
web/src/beta/lib/reearth-ui/components/ColorInput/index.tsx (3)
8-8
: LGTM: Import statement properly placed
The useT hook import is correctly added and follows the existing import organization pattern.
50-50
: LGTM: Hook initialization follows best practices
The useT hook is properly initialized before the render function, following React hooks best practices.
78-86
: 🛠️ Refactor suggestion
Add translation for PopupPanel title
While the button titles are properly internationalized, the PopupPanel title "Color Picker" is still hardcoded. Consider updating it to use the translation system for consistency.
Apply this change:
<PopupPanel
- title="Color Picker"
+ title={t("ColorPicker")}
onCancel={handlePickerCancel}
Let's verify the translation keys exist:
web/src/beta/lib/reearth-ui/components/Selector/index.tsx (3)
1-1
: LGTM: Import of i18n hook
The addition of the useT
hook import is appropriate for implementing internationalization support.
34-34
: LGTM: Placeholder prop modification
Removing the default value from the prop definition is appropriate as it allows for more flexible internationalization handling.
40-40
: LGTM: Translation hook initialization
The useT
hook is properly initialized and follows React hook conventions.
web/src/beta/features/AssetsManager/hooks.ts (2)
239-239
: LGTM: Translation integration looks good!
The translation of the "Assets" string using the useT
hook aligns with the PR's internationalization objectives and follows the established pattern across the codebase.
239-239
: Verify translation key availability
Let's ensure the "Assets" translation key is properly defined in the localization files.
✅ Verification successful
Translation key 'Assets' is properly defined in localization files
The verification shows that the "Assets" translation key is correctly defined in both English (en.yml) and Japanese (ja.yml) translation files:
web/src/services/i18n/translations/en.yml
web/src/services/i18n/translations/ja.yml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the "Assets" translation key exists in all locale files
# Search for "Assets" key in all locale files
echo "Checking translation files for 'Assets' key..."
fd -e yml -e yaml . | xargs rg "Assets:" -A 1
# Check manifest files specifically as mentioned in the PR summary
echo "Checking manifest files..."
fd manifest_ -e yml -e yaml . | xargs rg "Assets:" -A 1
Length of output: 748
web/src/services/i18n/translations/en.yml (3)
185-185
: Verify consistent usage of primary action terms
The term "Create" is used as a primary action. Let's verify its consistent usage across different contexts in the codebase.
#!/bin/bash
# Search for variations of create/new/add actions
rg -i "^(create|new|add).*:" web/src/services/i18n/translations/
367-368
: Ensure consistent UI state messages
The loading and selection prompts should be consistent across the application. Let's verify their usage.
#!/bin/bash
# Search for loading and selection states
rg -i "^(loading|please|select).*:" web/src/services/i18n/translations/
221-221
: Verify consistent navigation terminology
The term "Pages" is used for navigation. Let's verify consistent terminology across navigation elements.
#!/bin/bash
# Search for navigation-related terms
rg -i "^(pages|layers|projects|assets):" web/src/services/i18n/translations/
web/src/services/i18n/translations/ja.yml (2)
20-34
: LGTM! Basic UI translations are accurate and consistent.
The translations follow common Japanese UI patterns and maintain consistency with standard terminology.
34-38
: Good improvement in terminology clarity!
The change from "パス" (Path) to "アセットURL" (Asset URL) makes the meaning clearer and more specific for users.
server/pkg/builtin/manifest_ja.yml (3)
1307-1307
: LGTM: Improved terminology
The change from 見えるレイヤー
to 表示するレイヤー
is a good improvement in Japanese terminology, making it more formal and clearer.
1329-1329
: LGTM: Clear documentation
Good addition of the description that clearly specifies the valid range for the gap value.
1406-1420
: LGTM: Well-structured new component
The new linkButtonStoryBlock
section is well-structured and contains all necessary fields for a link button component, following the established pattern of other story blocks.
web/src/beta/features/Dashboard/ContentsContainer/Projects/Project/ProjectListViewItem.tsx
Show resolved
Hide resolved
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: 7
🧹 Outside diff range and nitpick comments (4)
web/src/beta/utils/time.ts (3)
22-24
: Handle future dates in time difference calculationThe current implementation doesn't handle future dates (when
date
is after the current time), which would result in negative seconds. Consider adding support for future dates to make the function more robust.export const formatRelativeTime = (date: Date, lang = "en"): string => { const seconds = Math.floor((new Date().getTime() - date.getTime()) / 1000); + const isNegative = seconds < 0; + const absoluteSeconds = Math.abs(seconds);
25-34
: LGTM! Consider extracting units to a separate configuration fileThe units definition is well-structured and type-safe. For better maintainability and easier addition of more languages in the future, consider extracting this to a separate i18n configuration file.
// i18n/timeUnits.ts export const timeUnits = { year: { value: 31536000, translations: { en: "year", ja: "年" } }, // ... other units };
37-46
: Consider using Array methods for better performanceThe current implementation using
for...in
could be more efficient using array methods. Also, consider extracting the pluralization logic to prepare for additional languages.- for (const unitKey in units) { - const { value, label, jaLabel } = units[unitKey]; - const interval = Math.floor(seconds / value); - if (interval >= 1) { + const unit = Object.entries(units).find(([_, { value }]) => { + const interval = Math.floor(absoluteSeconds / value); + return interval >= 1; + }); + + if (unit) { + const [_, { value, label, jaLabel }] = unit; + const interval = Math.floor(absoluteSeconds / value); const unitLabel = lang === "ja" ? jaLabel : label + (interval > 1 && lang !== "ja" ? "s" : ""); - return `${interval} ${unitLabel}${lang === "ja" ? " 前" : " ago"}`; - } + const suffix = isNegative + ? (lang === "ja" ? "後" : "from now") + : (lang === "ja" ? "前" : "ago"); + return `${interval} ${unitLabel} ${suffix}`; + }web/src/beta/utils/time.test.ts (1)
5-75
: Consider using test.each for more maintainable testsThe test suite contains repetitive test cases that follow the same pattern. This could be refactored using
test.each
to make the tests more maintainable and concise.Consider refactoring the tests like this:
describe("formatRelativeTime", () => { const now = new Date(); test.each([ { lang: "en", expected: "just now" }, { lang: "ja", expected: "たった今" }, ])("returns current time in $lang", ({ lang, expected }) => { expect(formatRelativeTime(now, lang)).toBe(expected); }); test.each([ { interval: { seconds: 10 }, en: "10 seconds ago", ja: "10 秒 前" }, { interval: { minutes: 5 }, en: "5 minutes ago", ja: "5 分 前" }, { interval: { hours: 3 }, en: "3 hours ago", ja: "3 時間 前" }, { interval: { days: 2 }, en: "2 days ago", ja: "2 日 前" }, // ... other intervals ])("returns $interval in different languages", ({ interval, en, ja }) => { const date = subtractFromDate(now, interval); expect(formatRelativeTime(date, "en")).toBe(en); expect(formatRelativeTime(date, "ja")).toBe(ja); }); }); // Helper function to calculate dates function subtractFromDate(date: Date, interval: Record<string, number>): Date { const result = new Date(date); if ("years" in interval) result.setFullYear(date.getFullYear() - interval.years); if ("months" in interval) result.setMonth(date.getMonth() - interval.months); if ("days" in interval) result.setDate(date.getDate() - interval.days); if ("hours" in interval) result.setHours(date.getHours() - interval.hours); if ("minutes" in interval) result.setMinutes(date.getMinutes() - interval.minutes); if ("seconds" in interval) result.setSeconds(date.getSeconds() - interval.seconds); return result; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
server/pkg/builtin/manifest_ja.yml
(3 hunks)web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx
(5 hunks)web/src/beta/ui/fields/Properties/ProprtyField.tsx
(3 hunks)web/src/beta/utils/time.test.ts
(1 hunks)web/src/beta/utils/time.ts
(1 hunks)web/src/services/i18n/translations/en.yml
(10 hunks)web/src/services/i18n/translations/ja.yml
(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx
- web/src/beta/ui/fields/Properties/ProprtyField.tsx
- web/src/services/i18n/translations/ja.yml
🧰 Additional context used
🪛 yamllint
server/pkg/builtin/manifest_ja.yml
[error] 1426-1426: no new line character at the end of file
(new-line-at-end-of-file)
[error] 1426-1426: trailing spaces
(trailing-spaces)
🔇 Additional comments (6)
web/src/beta/utils/time.ts (1)
22-49
: Verify test coverage for the updated function
Let's verify the test coverage for the formatRelativeTime
function, especially for the new language support.
✅ Verification successful
Test coverage for formatRelativeTime
is comprehensive and well-implemented
The test file web/src/beta/utils/time.test.ts
includes thorough test coverage for the formatRelativeTime
function:
- Tests for both English and Japanese language support
- Coverage for all time units (seconds, minutes, hours, days, months, years)
- Tests for plural forms in English
- Tests for "just now" case in both languages
- Tests for proper suffix handling ("ago" in English, "前" in Japanese)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check test coverage for formatRelativeTime function
# Look for test file
echo "Searching for test file..."
fd "time.test" --type f
# Check test cases in the file
echo -e "\nChecking test cases..."
rg -A 5 "formatRelativeTime" --type ts
Length of output: 9133
web/src/beta/utils/time.test.ts (1)
57-58
: 🛠️ Refactor suggestion
Improve month calculation accuracy
The current implementation uses a fixed 30-day month calculation which may lead to inaccurate results. Months have varying lengths (28-31 days), and this simplification could cause edge cases.
Consider using a more accurate month calculation:
- const date = new Date(now.getTime() - 3 * 30 * 24 * 60 * 60 * 1000); // 3 months ago
+ const threeMonthsAgo = new Date(now);
+ threeMonthsAgo.setMonth(now.getMonth() - 3);
Also applies to: 62-63
web/src/services/i18n/translations/en.yml (1)
67-67
: Verify usage of new translation keys
Let's verify that all newly added translation keys are actually used in the codebase.
Also applies to: 185-185, 221-221, 369-370, 394-397
✅ Verification successful
All translation keys are properly used in the codebase
The verification shows that all translation keys are actively used across the codebase:
- "Remove" in SelectableArea, Members, and ProjectRemoveModal components
- "Create" in SketchLayerCreator and CreateWorkspaceModal
- "Pages" in PagesPanel
- "Please select" in Selector component
- "New Item" in ListField
- "New Credit" in ListItem
- "min" and "max" in ProprtyField
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of new translation keys in the codebase
# Search for translation key usage
echo "Checking translation key usage..."
for key in "Remove" "Create" "Pages" "Loading" "Please select" "New Item" "New Credit" "min" "max"; do
echo "=== Searching for key: $key ==="
rg -i "useT.*['\"]${key}['\"]" web/src/
rg -i "t\\(['\"]${key}['\"]\\)" web/src/
done
Length of output: 3633
server/pkg/builtin/manifest_ja.yml (3)
1307-1307
: LGTM: Field name updated for better clarity in Japanese.
The change from showLayers
to 表示するレイヤー
improves clarity for Japanese users by using a more descriptive term.
1329-1329
: LGTM: Added helpful description for gap parameter.
The added description clearly specifies the valid range (min: 0, max: 40) for the gap parameter.
1410-1424
: LGTM: Well-structured new linkButtonStoryBlock section.
The new section is well-organized with clear field definitions for title, colors, and URL.
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: 2
🧹 Outside diff range and nitpick comments (1)
web/src/beta/utils/time.ts (1)
22-34
: Consider enhancing type safety and maintainability.The implementation could benefit from the following improvements:
- Use an enum for supported languages
- Use a const assertion for the units object
- Add TypeScript utility types for better type inference
+type SupportedLanguage = "en" | "ja"; + +const units = { year: { value: 31536000, label: "year", jaLabel: "年" }, month: { value: 2592000, label: "month", jaLabel: "ヶ月" }, day: { value: 86400, label: "day", jaLabel: "日" }, hour: { value: 3600, label: "hour", jaLabel: "時間" }, minute: { value: 60, label: "minute", jaLabel: "分" }, second: { value: 1, label: "second", jaLabel: "秒" } -} as const; +} as const satisfies Record<string, { value: number; label: string; jaLabel: string }>; + -export const formatRelativeTime = (date: Date, lang = "en"): string => { +export const formatRelativeTime = (date: Date, lang: SupportedLanguage = "en"): string => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
web/src/beta/utils/time.test.ts
(1 hunks)web/src/beta/utils/time.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/beta/utils/time.test.ts
🔇 Additional comments (1)
web/src/beta/utils/time.ts (1)
53-53
: Fix typo in Japanese text (たっだ今 → たった今).
This issue was previously reported. The Japanese text for "just now" contains a typo:
- return lang === "ja" ? "たっだ今" : "just now";
+ return lang === "ja" ? "たった今" : "just now";
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.
I checked Japanese translation, LTGM!
Overview
What I've done
Check UI and fix language issue
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Summary by CodeRabbit
Release Notes
New Features
linkButtonStoryBlock
for enhanced storytelling configuration.Improvements
Bug Fixes
Documentation