refactor(web): sketch layer and the custom properties #1414
Conversation
…actor/sketchlayer-customproperties
…actor/sketchlayer-customproperties
WalkthroughThis pull request implements extensive enhancements to custom property management in the mapping editor. New React components and hooks are added to create, edit, and delete custom properties, while obsolete functionality (e.g., the SketchLayerEditor and General component) is removed. The PR introduces collapsible UI sections, confirmation modals, and state management improvements, along with corresponding API mutations and updated translation keys for a better, more streamlined user interface. Changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 YAMLlint (1.35.1)web/src/services/i18n/translations/en.yml[Errno 2] No such file or directory: 'web/src/services/i18n/translations/en.yml' web/src/services/i18n/translations/ja.yml[Errno 2] No such file or directory: 'web/src/services/i18n/translations/ja.yml' ✨ 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
Documentation and Community
|
✅ Deploy Preview for reearth-web ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (15)
web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx (2)
50-57: Consider using a more robust sorting mechanism.The current schema generation logic relies on array indices for sorting, which could be fragile if the array order changes during operations like filtering or reordering.
Consider using a dedicated order field in the CustomPropertyProp type:
const schemaJSON = customProperties.reduce((acc, property, index) => { const [key] = Object.keys(property); - // Appending index + 1 to the value for sorting later - const value = `${property[key]}_${index + 1}`; + // Use a dedicated order field from the property + const value = `${property[key]}_${property.order}`; acc[key] = value; return acc; }, {});
98-102: Add input validation for layer name.The layer name input lacks validation for format and length constraints.
Consider adding validation:
<TextInput placeholder={t(" Text")} value={layerName} + maxLength={50} + pattern="^[a-zA-Z0-9_-]+$" onChange={handleLayerNameChange} + onInvalid={(e) => e.target.setCustomValidity(t("Layer name can only contain letters, numbers, underscores, and hyphens"))} />web/src/beta/ui/components/ConfirmModal/index.tsx (1)
10-15: Consider adding accessibility props.The
ConfirmModalProptype could benefit from additional accessibility-related props:type ConfirmModalProp = { visible: boolean; description?: string; title?: string; actions?: ReactNode; + ariaLabel?: string; + ariaDescribedBy?: string; + onClose?: () => void; };web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldItem.tsx (2)
26-28: Consider memoizing handleOptionsClick with dependencies.While
useCallbackis used, the empty dependency array suggests this could be a static function:-const handleOptionsClick = useCallback((e: MouseEvent) => { +const handleOptionsClick = (e: MouseEvent) => { e.stopPropagation(); -}, []); +};
31-44: Memoize optionsMenu to prevent unnecessary re-renders.The options menu array is recreated on every render:
-const optionsMenu: PopupMenuItem[] = [ +const optionsMenu = useMemo<PopupMenuItem[]>(() => [ { id: "edit", title: t("Edit Field"), icon: "pencilSimple" as const, onClick: openCustomPropertySchema }, { id: "delete", title: t("Delete Field"), icon: "trash" as const, onClick: () => onDeleteField(title) } -]; +], [t, openCustomPropertySchema, onDeleteField, title]);web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/index.tsx (2)
22-23: Consider using a more specific localStorage key prefix.The current key prefix
reearth-visualizer-infois quite generic. Consider using a more specific prefix likereearth-visualizer-layer-inspectorto avoid potential conflicts with other components.- localStorage.getItem(`reearth-visualizer-info-${id}-collapsed`) === + localStorage.getItem(`reearth-visualizer-layer-inspector-${id}-collapsed`) ===
103-113: Consider memoizing styled components.The styled components
WrapperandInputWrapperdepend on the theme object. Consider memoizing them usinguseMemoto prevent unnecessary re-renders when parent components update.-const Wrapper = styled("div")(({ theme }) => ({ +const Wrapper = useMemo(() => styled("div")(({ theme }) => ({ display: "flex", flexDirection: "column", gap: theme.spacing.small -})); +})), []); -const InputWrapper = styled("div")(({ theme }) => ({ +const InputWrapper = useMemo(() => styled("div")(({ theme }) => ({ display: "flex", flexDirection: "column", gap: theme.spacing.large -})); +})), []);web/src/beta/features/Editor/Map/SketchLayerCreator/SketchCustomProperties/index.tsx (1)
98-100: Make the warning message more specific.The current warning message is quite generic. Consider making it more specific by mentioning which keyword is causing the conflict.
- "The keyword you want to use as the custom property title has been used in the system, please choose any other keyword" + `The title "${customProperties?.title}" is already in use. Please choose a different title.`web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx (1)
63-69: Add aria-label to improve accessibility.The "New Field" button could benefit from a more descriptive aria-label for screen readers.
<Button icon="plus" title={t("New Field")} + aria-label={t("Add new custom property field")} size="small" onClick={openCustomPropertySchema} />web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx (2)
35-70: Consider enhancing data type definitions for better maintainability.The data types and groups could be improved by:
- Using an enum for data type IDs
- Creating a single source of truth by deriving groups from types
-const dataTypes = [ +enum DataTypeId { + TEXT = "text", + TEXT_AREA = "textArea", + URL = "url", + ASSET = "asset", + FLOAT = "float", + INT = "int", + BOOL = "bool" +} + +const dataTypes = [ { - id: "text", + id: DataTypeId.TEXT, title: "Text" }, // ... other types ]; -const dataTypeGroups = { +const dataTypeGroups = Object.freeze({ string: ["Text", "TextArea", "URL", "Asset"], number: ["Float", "Int"], boolean: ["Boolean"] -}; +});
130-178: Enhance modal accessibility.The modal component could benefit from improved accessibility features:
<Modal size="small" visible={true}> <ModalPanel + role="dialog" + aria-labelledby="modal-title" + aria-describedby="modal-description" title={ - isEditField ? t("Edit Custom Property") : t("New Custom Property") + <h2 id="modal-title"> + {isEditField ? t("Edit Custom Property") : t("New Custom Property")} + </h2> } onCancel={onClose} actions={/* ... */} > <Wrapper> <InputGroup label={t("Property Title")}> <InputsWrapper> <TextInput + aria-required="true" + aria-invalid={!customPropertyTitle?.trim()} value={customPropertyTitle} onChange={setCustomPropertyTitle} onBlur={onBlur} /> </InputsWrapper> </InputGroup> {/* ... */} </Wrapper> </ModalPanel> </Modal>web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts (1)
8-22: Consider using TypeScript enum for icon mapping.The icon mapping could be more type-safe and maintainable:
+enum PropertyType { + TEXT_AREA = "TextArea", + TEXT = "Text", + URL = "URL", + ASSET = "Asset", + FLOAT = "Float", + INT = "Int", + BOOLEAN = "Boolean", + CAMERA = "Camera" +} export const getIcon = (value: string): IconName => { const cleanedValue = value.replace(/_\d+$/, ""); - const iconMap: Record<string, IconName> = { - TextArea: "textarea", + const iconMap: Record<PropertyType, IconName> = { + [PropertyType.TEXT_AREA]: "textarea", // ... rest of the mapping }; return iconMap[cleanedValue as PropertyType] || "file"; };web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx (1)
182-203: Consider extracting modal content to a separate component.The ConfirmModal content could be extracted to improve maintainability:
+const DeleteLayerConfirmModal: FC<{ + isVisible: boolean; + onClose: () => void; + onConfirm: () => void; + layerTitle: string; +}> = ({ isVisible, onClose, onConfirm, layerTitle }) => { + const t = useT(); + return ( + <ConfirmModal + visible={isVisible} + title={t("Delete this Layer?")} + description={t( + "Are you sure you want to remove this Sketch Layer? If deleted, all data of this layer will be lost and you can not recover it again." + )} + actions={ + <> + <Button size="normal" title={t("Cancel")} onClick={onClose} /> + <Button + size="normal" + title={t("Delete")} + appearance="dangerous" + onClick={onConfirm} + /> + </> + } + /> + ); +};web/src/services/api/layersApi/index.ts (1)
207-238: Consider extracting common mutation logic to reduce duplication.The mutation handling logic is repeated across multiple functions. Consider creating a helper:
+const createMutationHandler = <T extends { [key: string]: any }>( + mutation: (options: any) => Promise<any>, + successKey: keyof T, + successMessage: string, + errorMessage: string +) => { + return async (input: { layerId: string } & Record<string, any>) => { + if (!input.layerId) return { status: "error" }; + const { data, errors } = await mutation({ + variables: { input } + }); + if (errors || !data?.[successKey]) { + setNotification({ + type: "error", + text: t(errorMessage) + }); + return { status: "error", errors }; + } + setNotification({ + type: "success", + text: t(successMessage) + }); + return { data, status: "success" }; + }; +}; -const useChangeCustomPropertyTitle = useCallback(/* ... */); +const useChangeCustomPropertyTitle = useCallback( + createMutationHandler( + changeCustomPropertyTitleMutation, + "changeCustomPropertyTitle", + "Successfully updated the custom property title!", + "Failed to update the custom property title." + ), + [changeCustomPropertyTitleMutation, setNotification, t] +);web/src/beta/features/Editor/hooks/index.ts (1)
1-353: Consider grouping related handlers together.The file has good modularity, but consider grouping related handlers (e.g., all custom property related handlers) together in both the destructuring and return object for better maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
web/src/services/gql/__gen__/gql.tsis excluded by!**/__gen__/**web/src/services/gql/__gen__/graphql.tsis excluded by!**/__gen__/**
📒 Files selected for processing (24)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldItem.tsx(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/index.tsx(1 hunks)web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx(4 hunks)web/src/beta/features/Editor/Map/LayersPanel/index.tsx(1 hunks)web/src/beta/features/Editor/Map/SketchLayerCreator/General/index.tsx(0 hunks)web/src/beta/features/Editor/Map/SketchLayerCreator/SketchCustomProperties/index.tsx(3 hunks)web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx(4 hunks)web/src/beta/features/Editor/Map/SketchLayerCreator/type.ts(2 hunks)web/src/beta/features/Editor/Map/SketchLayerEditor/hooks.ts(0 hunks)web/src/beta/features/Editor/Map/SketchLayerEditor/index.tsx(0 hunks)web/src/beta/features/Editor/Map/context.tsx(2 hunks)web/src/beta/features/Editor/hooks/index.ts(3 hunks)web/src/beta/features/Editor/hooks/useLayers.ts(4 hunks)web/src/beta/features/Editor/hooks/useUI.ts(0 hunks)web/src/beta/features/Editor/index.tsx(0 hunks)web/src/beta/lib/reearth-ui/components/ModalPanel/index.tsx(1 hunks)web/src/beta/ui/components/ConfirmModal/index.tsx(1 hunks)web/src/services/api/layersApi/index.ts(3 hunks)web/src/services/gql/queries/layer.ts(1 hunks)web/src/services/i18n/translations/en.yml(5 hunks)web/src/services/i18n/translations/ja.yml(5 hunks)
💤 Files with no reviewable changes (5)
- web/src/beta/features/Editor/hooks/useUI.ts
- web/src/beta/features/Editor/Map/SketchLayerCreator/General/index.tsx
- web/src/beta/features/Editor/index.tsx
- web/src/beta/features/Editor/Map/SketchLayerEditor/index.tsx
- web/src/beta/features/Editor/Map/SketchLayerEditor/hooks.ts
✅ Files skipped from review due to trivial changes (2)
- web/src/beta/lib/reearth-ui/components/ModalPanel/index.tsx
- web/src/beta/features/Editor/Map/LayersPanel/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-web / ci
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
🔇 Additional comments (15)
web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx (2)
5-5: LGTM! Enhanced data type support and UI components.The addition of Boolean and Camera data types expands the sketch layer functionality, while the new UI components align with the simplified interface design.
Also applies to: 11-11, 28-29
119-126: LGTM! Clean and consistent styling.The updated styling uses modern flex layout and maintains consistency with the theme system.
web/src/beta/features/Editor/Map/SketchLayerCreator/type.ts (2)
12-13: LGTM! Addition of Camera type expands functionality.The addition of the "Camera" type to
SketchLayerDataTypeenhances the layer creation capabilities.
30-34: Verify the removal of layerStyles property.The removal of
layerStylesfromSketchLayerPropssuggests a change in how layer styles are managed. Please ensure that layer styles are properly handled in the new architecture.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for references to layerStyles to ensure the removal doesn't break existing code rg -A 3 "layerStyles"Length of output: 25966
Layer styles removal from SketchLayerProps is consistent with the new architecture
The removal of the layerStyles property from SketchLayerProps appears intentional and is validated by our investigation. Layer styles are still actively managed but through dedicated components, hooks, and context (for example, within the LayerStylePanel, PresetLayerStyle components, and via the useLayerStyles hook). There’s no evidence from the codebase that SketchLayerCreator expects the layerStyles prop—its responsibilities now rely on sceneId, onClose, and onSubmit.
• SketchLayerCreator no longer references layerStyles, and the type update is aligned with the new management strategy.
• Other parts of the codebase continue to use layerStyles appropriately, confirming that the change is isolated and supported by alternative implementations.web/src/services/gql/queries/layer.ts (2)
51-59: LGTM! Well-structured mutation for changing custom property title.The mutation follows the established pattern and includes proper error handling through GraphQL.
61-69: LGTM! Well-structured mutation for removing custom property.The mutation is properly defined and follows the same pattern as other mutations in the file.
web/src/beta/features/Editor/Map/context.tsx (1)
67-70: LGTM! Well-structured interface updates.The new methods for managing custom properties are well-typed and align with the PR objectives.
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx (1)
36-43: Add error handling for custom property operations.The hook usage doesn't include error handling for failed operations. Consider adding error handling and user feedback.
web/src/beta/features/Editor/hooks/index.ts (2)
52-54: LGTM! New custom property handlers added consistently.The new handlers for managing custom property titles and removal are properly integrated into the useLayers hook.
191-193: LGTM! Consistent updates to mapPageValue.The new custom property handlers are properly added to both the mapPageValue object and its dependencies array, maintaining consistency.
Also applies to: 230-232
web/src/beta/features/Editor/hooks/useLayers.ts (3)
9-13: LGTM! Well-organized type imports.The custom property related types are properly imported from the GraphQL schema.
308-329: LGTM! Well-implemented custom property handlers.The new handlers follow consistent patterns and properly utilize the mutation hooks for managing custom properties.
344-346: LGTM! Consistent return value updates.The new handlers are properly exposed in the return object.
web/src/services/i18n/translations/en.yml (2)
124-124: LGTM! Clear and consistent UI translations.The new translations for custom property management follow good UX writing principles and maintain consistency with existing translations.
Also applies to: 127-143
428-434: LGTM! Well-structured feedback messages.The success and failure messages for custom property operations are clear and follow the established pattern.
.../InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldItem.tsx
Outdated
Show resolved
Hide resolved
...InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx
Outdated
Show resolved
Hide resolved
...features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts
Outdated
Show resolved
Hide resolved
…actor/sketchlayer-customproperties
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts (2)
8-22: Consider moving iconMap outside the function.Moving iconMap outside the function would prevent it from being recreated on each call, improving performance.
+const iconMap: Record<string, IconName> = { + TextArea: "textarea", + Text: "textT", + URL: "paperclip", + Asset: "image", + Float: "file", + Int: "file", + Boolean: "file", + Camera: "camera" +}; export const getIcon = (value: string): IconName => { const cleanedValue = value.replace(/_\d+$/, ""); // Remove _Number suffix - const iconMap: Record<string, IconName> = { - TextArea: "textarea", - Text: "textT", - URL: "paperclip", - Asset: "image", - Float: "file", - Int: "file", - Boolean: "file", - Camera: "camera" - }; return iconMap[cleanedValue] || "file"; };
184-208: Consider splitting handleSubmit into separate functions.The function handles both title updates and schema updates. Splitting it would improve maintainability and make the code easier to test.
+const handleTitleUpdate = useCallback(() => { + if (!customPropertySchema || !selectedField?.key || !newTitle) return; + const { [selectedField.key]: _, ...updatedSchema } = schemaJSON; + updatedSchema[newTitle] = schemaJSON[selectedField.key]; + setSchemaJSON(updatedSchema); + handleUpdateCustomPropertyTitle(updatedSchema); + closeEditFieldConfirmModal(); +}, [customPropertySchema, selectedField?.key, newTitle, schemaJSON, closeEditFieldConfirmModal, handleUpdateCustomPropertyTitle]); + +const handleSchemaUpdate = useCallback(() => { + if (!customPropertySchema || !selectedField?.key) return; + handleUpdateCustomPropertySchema(schemaJSON); + closeEditFieldConfirmModal(); +}, [customPropertySchema, selectedField?.key, schemaJSON, closeEditFieldConfirmModal, handleUpdateCustomPropertySchema]); + const handleSubmit = useCallback(() => { - if (!customPropertySchema || !selectedField?.key) return; - - const { [selectedField.key]: _, ...updatedSchema } = schemaJSON; - - if (newTitle) { - updatedSchema[newTitle] = schemaJSON[selectedField.key]; - setSchemaJSON(updatedSchema); - handleUpdateCustomPropertyTitle(updatedSchema); - } - - if (!newTitle) { - handleUpdateCustomPropertySchema(schemaJSON); - } - - closeEditFieldConfirmModal(); + newTitle ? handleTitleUpdate() : handleSchemaUpdate(); }, [/* ... */]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: ci-web / ci
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-server / ci-server-test
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
🔇 Additional comments (5)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx (3)
1-16: LGTM!Props and imports are well-organized and properly typed.
18-44: LGTM!Clean hook usage with well-organized state management.
127-153: LGTM!Styled components are well-organized with proper theme usage and layout.
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts (2)
24-52: LGTM!State management is well-organized with proper dependency tracking.
121-128: LGTM!The implementation correctly uses Promise.resolve().then() instead of setTimeout(0) as suggested in the previous review.
...eatures/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx
Outdated
Show resolved
Hide resolved
...features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx (3)
98-98: Remove unnecessary leading space in placeholder text.The placeholder text has an extra leading space which is unnecessary.
- placeholder={t(" Text")} + placeholder={t("Text")}
48-74: Enhance type safety and readability of schema generation.The schema generation logic could benefit from better type safety and readability improvements.
Consider this refactoring:
const handleSubmit = () => { - const schemaJSON = customProperties.reduce((acc, property, index) => { + const schemaJSON: Record<string, string> = customProperties.reduce((acc, property, index) => { const [key] = Object.keys(property); - // Appending index + 1 to the value for sorting later - const value = `${property[key]}_${index + 1}`; + // Append index for maintaining property order + const propertyValue = property[key]; + const sortableValue = `${propertyValue}_${index + 1}`; acc[key] = value; return acc; - }, {}); + }, {} as Record<string, string>);
89-89: Consider adding a tooltip for disabled state.The create button is disabled when there's no layer name or when there's a warning, but there's no visual feedback explaining why.
disabled={!layerName || warning} + tooltip={!layerName ? t("Layer name is required") : warning ? t("Please fix the warnings in custom properties") : undefined}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
- GitHub Check: ci-web / ci
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-server / ci-server-test
🔇 Additional comments (2)
web/src/beta/features/Editor/Map/SketchLayerCreator/index.tsx (2)
1-29: LGTM! Clean import organization and comprehensive data types.The import statements are well-organized, and the data types array provides a good range of supported types for sketch layer properties.
118-125: LGTM! Well-structured theme-aware styling.The styling implementation follows best practices by:
- Using theme variables consistently
- Implementing a clean flex layout
- Maintaining proper spacing using theme values
...eatures/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx
Outdated
Show resolved
Hide resolved
...eatures/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx
Outdated
Show resolved
Hide resolved
...eatures/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx
Outdated
Show resolved
Hide resolved
…actor/sketchlayer-customproperties
…actor/sketchlayer-customproperties
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx (2)
53-63: Consider making the flyTo delay configurable.The hardcoded 800ms delay in the
handleZoomToLayerfunction might not work consistently across different network conditions, as noted in the comment. Consider making this delay configurable or implementing a more robust solution.+const ZOOM_DELAY = 800; // ms + const handleZoomToLayer = useCallback(() => { handleFlyTo?.(layer.id, { duration: 0 }); // issue: https://github.com/CesiumGS/cesium/issues/4327 // delay 800ms to trigger a second flyTo, // time could be related with internet speed, not a stable solution if (["geojson", "kml"].includes(layer.config?.data?.type)) { setTimeout(() => { handleFlyTo?.(layer.id, { duration: 0 }); - }, 800); + }, ZOOM_DELAY); } }, [layer, handleFlyTo]);
183-206: Remove redundant visible prop from ConfirmModal.The
visibleprop is hardcoded totruebut is unnecessary since the modal is already conditionally rendered using&&operator.{showDeleteLayerConfirmModal && ( <ConfirmModal - visible={true} title={t("Delete this Layer?")} description={t( "Are you sure you want to remove this Layer? If deleted, all data of this layer will be lost and you can not recover it again." )} actions={web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx (1)
170-219: Add ARIA labels for better accessibility.The modal and form elements lack proper ARIA labels for screen readers.
Apply this diff to improve accessibility:
<Modal size="small" visible={true}> <ModalPanel + role="dialog" + aria-labelledby="modal-title" title={ - isEditField ? t("Edit Custom Property") : t("New Custom Property") + <h2 id="modal-title"> + {isEditField ? t("Edit Custom Property") : t("New Custom Property")} + </h2> } onCancel={onClose} actions={/* ... */} > <Wrapper> - <InputGroup label={t("Property Title")}> + <InputGroup label={t("Property Title")} aria-label={t("Property Title")}> <InputsWrapper> <TextInput value={customPropertyTitle} onChange={setCustomPropertyTitle} onBlur={onBlur} + aria-required="true" /> </InputsWrapper> </InputGroup> - <InputGroup label={t("Property Type")}> + <InputGroup label={t("Property Type")} aria-label={t("Property Type")}> <PopupMenu extendTriggerWidth extendContentWidth menu={menuItems} iconColor={theme.content.main} + aria-required="true" label={/* ... */} /> </InputGroup> </Wrapper> </ModalPanel> </Modal>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
web/src/beta/lib/reearth-ui/components/Icon/Icons/Float.svgis excluded by!**/*.svgweb/src/beta/lib/reearth-ui/components/Icon/Icons/LinkSimpleHorizontal.svgis excluded by!**/*.svgweb/src/beta/lib/reearth-ui/components/Icon/Icons/NumberNine.svgis excluded by!**/*.svgweb/src/beta/lib/reearth-ui/components/Icon/Icons/ToggleLeft.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldItem.tsx(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx(1 hunks)web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx(4 hunks)web/src/beta/features/Editor/Map/SketchLayerCreator/type.ts(2 hunks)web/src/beta/lib/reearth-ui/components/Icon/icons.ts(8 hunks)web/src/services/i18n/translations/en.yml(5 hunks)web/src/services/i18n/translations/ja.yml(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/src/beta/features/Editor/Map/SketchLayerCreator/type.ts
- web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldItem.tsx
- web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-server / ci-server-test
- GitHub Check: ci-web / ci
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
🔇 Additional comments (8)
web/src/beta/features/Editor/Map/LayersPanel/LayerItem.tsx (2)
1-31: LGTM!The imports are correctly added for the new confirmation modal feature, and the component props are well-defined.
69-82: LGTM!The options menu is well-structured with proper type safety and internationalization support. The addition of a confirmation step for deletion improves the user experience.
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx (1)
150-168: Enhance form validation and error handling.The handleSubmit function needs more robust validation and error handling.
Apply this diff to improve validation:
const handleSubmit = useCallback(() => { - if (!customPropertyTitle) return; + if (!customPropertyTitle?.trim()) { + // Consider showing an error message to the user + return; + } + if (!dataType) { + // Consider showing an error message to the user + return; + } onSchemaJSONUpdate?.((prevSchema = {}) => { try { const updatedSchema = { ...prevSchema }; updatedSchema[customPropertyTitle] = `${dataType}_${Object.keys(updatedSchema).length + 1}`; onCustomPropertySchemaUpdate?.(updatedSchema); return updatedSchema; + } catch (error) { + // Consider showing an error message to the user + console.error('Failed to update schema:', error); + return prevSchema; } }); onClose?.(); }, [/* ... */]);web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts (1)
53-62: Add error handling for malformed values in sorting logic.The current implementation assumes values will always have a valid _number suffix format.
Apply this diff to handle malformed values gracefully:
const sortedValues = useMemo(() => { if (!customPropertySchema) return; return Object.entries(customPropertySchema) .map(([key, value]) => ({ key, value })) .sort((a, b) => { - const aIndex = parseInt(a.value.split("_")[1]) || 0; - const bIndex = parseInt(b.value.split("_")[1]) || 0; + try { + const aIndex = parseInt(a.value.split("_")[1]) || 0; + const bIndex = parseInt(b.value.split("_")[1]) || 0; + return aIndex - bIndex; + } catch (error) { + console.warn(`Invalid value format: ${a.value} or ${b.value}`); + return 0; + } - return aIndex - bIndex; }); }, [customPropertySchema]);web/src/beta/lib/reearth-ui/components/Icon/icons.ts (1)
67-67: LGTM! New icons follow the existing pattern.The new icon imports and exports are consistent with the existing codebase structure.
Also applies to: 88-88, 104-104, 158-158, 235-235, 256-256, 272-272, 326-326
web/src/services/i18n/translations/en.yml (1)
124-141: LGTM! Comprehensive translations for custom properties.The new translations provide clear and helpful messages for users, including success/failure states and confirmation dialogs.
Also applies to: 156-157, 428-431
web/src/services/i18n/translations/ja.yml (2)
127-134: Missing Japanese translations need to be added.Several important translations are missing for the custom property management and layer features:
- Data source
- Custom property schema
- Edit/Delete Field
- Edit/New Custom Property
- Property Title/Type
- Delete Layer confirmation
- Custom property operation messages
As suggested by @airslice, consider using translation tools for initial translations and requesting review from @asahina820.
Also applies to: 136-138, 140-143, 159-160, 431-434
183-185: LGTM! New translations are consistent.The translations for sketch layer functionality are appropriate and consistent with the application's terminology:
- "New Sketch Layer" → "新しいスケッチレイヤー"
- "Create" → "作成"
- "Text" → "テキスト"
...features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts
Outdated
Show resolved
Hide resolved
…actor/sketchlayer-customproperties
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/src/services/i18n/translations/ja.yml (1)
431-434:⚠️ Potential issueAdd missing translations for custom property operations.
The following translations are missing:
- Failed/success messages for custom property operations
Apply this diff to add the translations:
-Failed to update the custom property title.: '' +Failed to update the custom property title.: 'カスタムプロパティのタイトルの更新に失敗しました。' -Successfully updated the custom property title!: '' +Successfully updated the custom property title!: 'カスタムプロパティのタイトルの更新に成功しました！' -Failed to remove the custom property.: '' +Failed to remove the custom property.: 'カスタムプロパティの削除に失敗しました。' -Successfully removed the custom property!: '' +Successfully removed the custom property!: 'カスタムプロパティの削除に成功しました！'
🧹 Nitpick comments (2)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts (2)
8-21: Enhance type safety in icon mapping.Consider using TypeScript enums or constants for icon names to improve type safety and maintainability.
+type CustomPropertyType = 'TextArea' | 'Text' | 'URL' | 'Asset' | 'Float' | 'Int' | 'Boolean'; + export const getIcon = (value: string): IconName => { const cleanedValue = value.replace(/_\d+$/, ""); // Remove _Number suffix - const iconMap: Record<string, IconName> = { + const iconMap: Record<CustomPropertyType, IconName> = { TextArea: "textarea", Text: "textT", URL: "linkSimpleHorizontal", Asset: "file", Float: "float", Int: "numberNine", Boolean: "toggleLeft" }; - return iconMap[cleanedValue] || "file"; + return iconMap[cleanedValue as CustomPropertyType] || "file"; };
132-132: Fix typo in function name.The function name "handleAppyDelete" contains a typo and should be "handleApplyDelete".
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts(1 hunks)web/src/services/i18n/translations/ja.yml(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
- GitHub Check: ci-web / ci
- GitHub Check: ci-server / ci-server-lint
🔇 Additional comments (5)
web/src/services/i18n/translations/ja.yml (2)
127-143: LGTM! New translations for custom property management are complete and accurate.The translations maintain consistency with the application's terminology and follow Japanese language conventions.
159-160: Missing translations were previously flagged.This issue was previously identified and it was suggested to use translation tools and get review from @asahina820.
Also applies to: 431-434
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts (3)
53-62: Add error handling for malformed values in sorting logic.The current implementation assumes values will always have a valid _number suffix format.
122-130: Well implemented modal management with proper microtask scheduling.The implementation correctly uses Promise.resolve() for scheduling the modal opening, avoiding the setTimeout anti-pattern.
212-234: Well-structured hook return value.The hook exports a comprehensive set of functions and state variables, making it easy for consumers to access all necessary functionality.
...features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts
Show resolved
Hide resolved
...features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx (1)
158-176:⚠️ Potential issueEnhance form validation and error handling.
The handleSubmit function still needs more robust validation and error handling as noted in the previous review.
const handleSubmit = useCallback(() => { - if (!customPropertyTitle) return; + if (!customPropertyTitle?.trim()) { + // Show error message to user + return; + } + if (!dataType) { + // Show error message to user + return; + } onSchemaJSONUpdate?.((prevSchema = {}) => { try { const updatedSchema = { ...prevSchema }; updatedSchema[customPropertyTitle] = `${dataType}_${Object.keys(updatedSchema).length + 1}`; onCustomPropertySchemaUpdate?.(updatedSchema); return updatedSchema; + } catch (error) { + console.error('Failed to update schema:', error); + // Show error message to user + return prevSchema; } }); onClose?.(); }, [onSchemaJSONUpdate, onCustomPropertySchemaUpdate, onClose, customPropertyTitle, dataType]);
🧹 Nitpick comments (3)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx (3)
22-34: Add JSDoc documentation for the props interface.Consider adding JSDoc documentation to improve code maintainability and developer experience.
+/** + * Props for the CustomPropertyField modal component + * @property {boolean} isEditField - Whether the modal is in edit mode + * @property {{ key: string; value: string } | null} selectedField - Currently selected field for editing + * @property {Record<string, string>} schemaJSON - Current schema object + * @property {(title: string) => void} onBlur - Callback when input loses focus + * @property {() => void} onClose - Callback to close the modal + * @property {(schema: CustomPropertyProp) => void} onCustomPropertySchemaUpdate - Callback to update custom property schema + * @property {() => void} onOpenConfirmModal - Callback to open confirmation modal + * @property {Dispatch<SetStateAction<Record<string, string>>>} onSchemaJSONUpdate - Callback to update schema JSON + */ type CustomPropertyFieldProps = { isEditField?: boolean; selectedField?: {
36-71: Move data type configurations to a separate constants file and improve type safety.Consider extracting these configurations to a dedicated constants file and using TypeScript enums or const assertions for better type safety.
Create a new file
constants.ts:export const enum DataType { Text = 'text', TextArea = 'textArea', URL = 'url', Asset = 'asset', Float = 'float', Int = 'int', Boolean = 'bool' } export const DATA_TYPES = [ { id: DataType.Text, title: 'Text' }, { id: DataType.TextArea, title: 'TextArea' }, { id: DataType.URL, title: 'URL' }, { id: DataType.Asset, title: 'Asset' }, { id: DataType.Float, title: 'Float' }, { id: DataType.Int, title: 'Int' }, { id: DataType.Boolean, title: 'Boolean' } ] as const; export const DATA_TYPE_GROUPS = { string: [DataType.Text, DataType.TextArea, DataType.URL, DataType.Asset], number: [DataType.Float, DataType.Int], boolean: [DataType.Boolean] } as const;
139-156: Simplify disabled state logic.The disabled state logic is complex and could be more readable by extracting it into smaller functions.
+const isPropertyTitleValid = (title: string | undefined) => !!title; + +const isDuplicateTitle = ( + title: string | undefined, + schema: Record<string, string> +) => title && Object.prototype.hasOwnProperty.call(schema, title); + +const hasTypeChanged = ( + currentType: string, + selectedType: string | undefined +) => currentType !== selectedType; + const disabled = useMemo(() => { - const checkExistValue = dataType !== selectedField?.value; + const typeChanged = hasTypeChanged(dataType, selectedField?.value); + const titleExists = isDuplicateTitle(customPropertyTitle, schemaJSON); return ( - !customPropertyTitle || - !dataType || - (!isEditField && - Object.prototype.hasOwnProperty.call(schemaJSON, customPropertyTitle)) || - (!checkExistValue && - Object.prototype.hasOwnProperty.call(schemaJSON, customPropertyTitle)) + !isPropertyTitleValid(customPropertyTitle) || + !dataType || + (!isEditField && titleExists) || + (!typeChanged && titleExists) ); }, [customPropertyTitle, dataType, schemaJSON, selectedField?.value, isEditField]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx(1 hunks)web/src/services/i18n/translations/ja.yml(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/services/i18n/translations/ja.yml
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: ci-server / ci-server-test
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-web / ci
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
🔇 Additional comments (1)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx (1)
231-252: LGTM! Well-structured styled components.The styled components make good use of theme values and have clear, semantic names.
...InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx
Outdated
Show resolved
Hide resolved
…actor/sketchlayer-customproperties
…actor/sketchlayer-customproperties
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts (2)
51-60:⚠️ Potential issueAdd error handling for malformed values in sorting logic.
The current implementation assumes values will always have a valid _number suffix format.
136-146:⚠️ Potential issueImprove type safety in handleUpdateCustomPropertySchema.
The function uses an empty string fallback for layerId, which could lead to invalid operations.
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx (1)
124-162:⚠️ Potential issueEnhance form validation and error handling.
The handleSubmit function needs more robust validation and error handling.
🧹 Nitpick comments (3)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx (1)
18-31: Consider using an enum for icon mapping.The
getIconfunction uses a string-based mapping which could be error-prone. Consider using an enum for better type safety and maintainability.+enum PropertyType { + TextArea = "TextArea", + Text = "Text", + URL = "URL", + Asset = "Asset", + Float = "Float", + Int = "Int", + Boolean = "Boolean" +} export const getIcon = (value: string): IconName => { const cleanedValue = value.replace(/_\d+$/, ""); // Remove _Number suffix - const iconMap: Record<string, IconName> = { + const iconMap: Record<PropertyType, IconName> = { TextArea: "textarea", Text: "textT", URL: "linkSimpleHorizontal", Asset: "file", Float: "float", Int: "numberNine", Boolean: "toggleLeft" }; - return iconMap[cleanedValue] || "file"; + return iconMap[cleanedValue as PropertyType] || "file"; };web/src/beta/features/Editor/Map/SketchLayerCreator/SketchCustomProperties/hooks.ts (1)
34-44: Enhance validation with early returns and clear error messages.The validation logic could be more explicit and provide better feedback.
const handleTitleBlur = useCallback( (idx: number) => (newKeyValue?: string) => { if (!propertiesList) return; + if (!newKeyValue?.trim()) { + setWarning?.(true); + return; + } const newList = propertiesList.map((i) => ({ ...i }) as PropertyListProp); newList[idx].key = newKeyValue?.trim() ?? ""; setPropertiesList?.(newList); const keyAlreadyExists = propertiesList.some( (item, index) => index !== idx && item.key === newKeyValue?.trim() ); const hasForbiddenKey = newList.some((item) => forbiddenKeywords.has(item.key) ); - if (hasForbiddenKey || keyAlreadyExists) { - setWarning?.(true); - } else { - setWarning?.(false); + if (hasForbiddenKey) { + setWarning?.(true); + console.warn(`Key "${newKeyValue}" is a reserved keyword`); + return; + } + if (keyAlreadyExists) { + setWarning?.(true); + console.warn(`Key "${newKeyValue}" already exists`); + return; } + setWarning?.(false); if (editTitleIndex === idx) setEditTitleIndex(null); }, [editTitleIndex, propertiesList, setPropertiesList, setWarning] );web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx (1)
207-218: Add type safety for Selector onChange.The onChange handler should use a more specific type than string[].
-onChange={(value: string | string[]) => - setDataType(value as string) -} +onChange={(value: string | string[]) => { + if (typeof value === 'string') { + setDataType(value); + } +}}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts(1 hunks)web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx(1 hunks)web/src/beta/features/Editor/Map/SketchLayerCreator/SketchCustomProperties/hooks.ts(1 hunks)web/src/services/i18n/translations/en.yml(5 hunks)web/src/services/i18n/translations/ja.yml(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: ci-server / ci-server-lint
- GitHub Check: ci-server / ci-server-test
- GitHub Check: ci-web / ci
- GitHub Check: Redirect rules - reearth-web
- GitHub Check: Header rules - reearth-web
- GitHub Check: Pages changed - reearth-web
🔇 Additional comments (8)
web/src/beta/features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/index.tsx (2)
63-71: Array indices used as React keys.Using array indices as keys in the map function can lead to performance issues and bugs with component state when items are reordered or deleted.
-{sortedValues?.map(({ key, value }) => ( +{sortedValues?.map(({ key, value }, idx) => ( <CustomPropertyFieldItem - key={key} + key={`${key}_${idx}`} title={key} icon={getIcon(value)} openCustomPropertySchema={() => handleEditField(key, value)} onDeleteField={handleDeleteField} /> ))}
92-115: Consider separating the ConfirmModal into a dedicated component.The ConfirmModal is reused but requires condition checks. Consider creating separate modals for different actions to improve code clarity.
web/src/services/i18n/translations/en.yml (3)
127-144: Custom Property Translation Entries Added.The new translations for custom properties—for keys such as "Data source," "Custom property schema," "Edit Field," "Delete Field," "Edit Custom Property," "New Custom Property," "Submit," "Property Title," "Property Type," "Please select one type," "Apply Current Edits?," "This save will apply to all features in the current layer. Do you want to proceed?," "No field has been created yet," "New Field," and "Delete schema property field?"/"This action will apply to all features in the current layer. Do you want to proceed?"—appear consistent and clear. Please verify that these keys are used appropriately in the UI.
160-161: Layer Deletion Confirmation Message Updated.The confirmation messages for deleting a layer now read:
- "Delete this Layer?: ''"
- "Are you sure you want to remove this Layer? If deleted, all data of this layer will be lost and you can not recover it again.: ''"
Ensure that these messages fully communicate the irreversible nature of the operation to the user and that they are consistent with other deletion prompts in the application.
180-182: Sketch Layer UI Text Translation Entries Added.The new entries "New Sketch Layer," "Create," and "' Text'" have been introduced. Make sure the placeholder "
' Text'" is intentional and fits the UI context (e.g., whether it should include surrounding quotes or not).web/src/services/i18n/translations/ja.yml (3)
127-144: Japanese Translations for Custom Property Management Added.The newly added translation entries for custom property functionality (including "Data source," "Custom property schema," "Edit Field," "Delete Field," "Edit Custom Property," "New Custom Property," "Submit," "Property Title," "Property Type," "Please select one type," "Apply Current Edits?," "This save will apply to all features in the current layer. Do you want to proceed?," "No field has been created yet," "New Field," and "Delete schema property field?") provide clear text for Japanese users. Please double-check that the terminology is consistent with other parts of the application.
160-161: Layer Deletion Confirmation Translations Updated.The messages for confirming layer deletion have been updated to:
- "Delete this Layer?: このレイヤーを削除しますか？"
- "Are you sure you want to remove this Layer? If deleted, all data of this layer will be lost and you can not recover it again.: 本当にこのレイヤーを削除しますか？削除すると、このレイヤーのデータはすべて失われ、二度と復元できません。"
This update aligns with previous feedback on Japanese translations for layer deletion.
183-185: Japanese Translations for Sketch Layer UI Added.The new entries "New Sketch Layer: 新しいスケッチレイヤー," "Create: 作成," and "' Text': ' テキスト'" have been added. Verify that these translations mesh well with the overall UI language and whether the quotes in "' Text'" are intentional.
...features/Editor/Map/InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/hooks.ts
Show resolved
Hide resolved
...InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx
Show resolved
Hide resolved
...InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx
Outdated
Show resolved
Hide resolved
...InspectorPanel/LayerInspector/DataSource/SketchCustomProperties/CustomPropertyFieldModal.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: lby <[email protected]>
Co-authored-by: lby <[email protected]>
Overview
I added changes in sketch layer especial sketch layer custom properties to much the current design
What I've done
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Summary by CodeRabbit
Summary by CodeRabbit
New Features
ConfirmModalcomponent for user confirmations.API Enhancements
Localization Updates