Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(web): add float field #1290

Merged
merged 12 commits into from
Nov 14, 2024
Merged

feat(web): add float field #1290

merged 12 commits into from
Nov 14, 2024

Conversation

caichi-t
Copy link
Contributor

@caichi-t caichi-t commented Oct 29, 2024

Overview

This PR adds the float field.

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a new Number field type with enhanced properties for numeric constraints.
    • Added new icons: infinity.
    • Expanded available field types in the "Number" category to include both "Integer" and "Number".
  • Bug Fixes

    • Improved validation logic in the NumberField component to ensure type checks for min and max values.
  • Documentation

    • Comprehensive updates to English and Japanese translations for improved clarity and consistency.
  • Refactor

    • Simplified rendering logic in multiple components, consolidating how fields are processed.
    • Updated InputNumber component for enhanced type safety.
    • Streamlined handling of GeometryObject and GeometryEditor field types across various components.
  • Chores

    • Removed deprecated field components from exports to streamline available options.

@caichi-t caichi-t requested a review from nourbalaha as a code owner October 29, 2024 07:46
Copy link

coderabbitai bot commented Oct 29, 2024

Walkthrough

This pull request introduces a variety of changes across multiple components in a project management application. It includes the addition of a test suite for float fields, updates to icon imports, modifications to input handling for number fields, and enhancements to the schema types. Notably, the IntegerField component is renamed to NumberField, and the handling of numeric types is consolidated throughout the application. Additionally, there are significant updates to translation files for both English and Japanese, reflecting a comprehensive overhaul of localization strings.

Changes

File Path Change Summary
web/e2e/project/item/fields/float.spec.ts Added a test suite for float fields with tests for creation, updating, and editing functionalities.
web/src/components/atoms/Icon/icons.ts Added InfinityIcon import; updated exports for new icons.
web/src/components/atoms/InputNumber/index.tsx Updated logic for determining input number status for type safety; added a style prop for width.
web/src/components/molecules/Common/Form/GroupItem/index.tsx Changed state type from Field[] to GroupField[]; simplified rendering logic for fields.
web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx Renamed IntegerField to NumberField; updated min/max logic to include new properties.
web/src/components/molecules/Content/Form/fields/FieldComponents/index.ts Removed exports for IntegerField, MarkdownField, SelectField, and TextAreaField.
web/src/components/molecules/Content/Form/fields/FieldTypesMap.ts Updated field type mappings to include NumberField; added mapping for Text field type.
web/src/components/molecules/Content/Form/index.tsx Simplified rendering logic by removing specific handling for GeometryObject and GeometryEditor.
web/src/components/molecules/Content/Table/DropdownRender/hooks.ts Updated filter type handling to consolidate "Integer" and "Number"; adjusted operator options.
web/src/components/molecules/Content/Table/DropdownRender/index.tsx Modified conditional rendering for InputNumber to include "Number".
web/src/components/molecules/Request/Details/ItemForm.tsx Simplified component rendering logic for geometry types using FIELD_TYPE_COMPONENT_MAP.
web/src/components/molecules/Schema/FieldList.tsx Updated "Number" category to include both "Integer" and "Number".
web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/index.tsx Replaced IntegerField handling with NumberField in switch statement.
web/src/components/molecules/Schema/FieldModal/FieldValidationInputs/index.tsx Expanded validation inputs to accommodate both "Integer" and "Number" types.
web/src/components/molecules/Schema/FieldModal/hooks.ts Updated min and max property handling with fallback values; added case for "Number" type.
web/src/components/molecules/Schema/fieldTypes.ts Introduced a new field type Number with properties for localization.
web/src/components/molecules/Schema/types.ts Updated schema types to include MetadataField, GroupField, and new properties for numeric constraints.
web/src/components/organisms/DataConverters/model.ts Updated fromGraphQLModel to cast fields as MetadataField.
web/src/components/organisms/DataConverters/schema.ts Updated fromGraphQLGroup to cast fields as GroupField.
web/src/gql/graphql-client-api.tsx Updated GraphQL query and mutation types to include numberMin and numberMax.
web/src/gql/queries/model.ts Added new fields to GET_MODEL_NODE query for numeric constraints.
web/src/i18n/translations/en.yml Complete replacement of English translation entries for UI strings.
web/src/i18n/translations/ja.yml Complete replacement of Japanese translation entries for UI strings.
web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/NumberField/index.tsx Renamed IntegerField to NumberField; updated validation logic for min/max checks.

Possibly related PRs

  • fix(web): option and tag field validation enhancement #1306: The changes in this PR enhance the validation process for option and tag fields, which is related to the main PR's focus on validating float fields in a project management application. Both involve testing and ensuring correct behavior for field inputs.

Suggested labels

server

Suggested reviewers

  • nourbalaha

🐇 In the garden, fields so bright,
A float field dances in the light.
With icons new and tests so grand,
We hop along, a joyful band!
Numbers now, with min and max,
Our code's a song, no need for hacks! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for reearth-cms ready!

Name Link
🔨 Latest commit 875125b
🔍 Latest deploy log https://app.netlify.com/sites/reearth-cms/deploys/67356583e8c87800082a3c41
😎 Deploy Preview https://deploy-preview-1290--reearth-cms.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🧹 Outside diff range and nitpick comments (21)
web/src/components/atoms/InputNumber/index.tsx (2)

17-17: Consider making width configurable.

While setting width:100% makes the component fill its container, this might not be desirable in all cases. Consider making it configurable:

-  return <AntDInputNumber value={value} status={status} style={{ width: "100%" }} {...props} />;
+  const defaultStyle = { width: "100%" };
+  return <AntDInputNumber value={value} status={status} style={{ ...defaultStyle, ...props.style }} {...props} />;

Line range hint 1-20: Consider adding precision validation.

Since this component is being added as part of a float field feature, consider adding validation for decimal precision:

  1. Add optional props for:
    • precision: number of decimal places
    • step: increment/decrement step size
  2. Consider implementing validation for scientific notation if needed
  3. Add proper error messages for validation failures

Example usage:

<InputNumber
  value={1.23}
  precision={2}
  step={0.1}
  min={0}
  max={100}
  onError={(msg) => console.error(msg)}
/>
web/src/components/molecules/Schema/FieldModal/FieldValidationInputs/index.tsx (1)

Line range hint 25-30: Consider adding step validation for Integer fields.

For better type safety, consider restricting the input to integers when selectedType === "Integer". This prevents users from entering decimal values for integer fields.

 <>
   <Form.Item name="min" label={t("Set minimum value")}>
-    <InputNumber type="number" max={max} />
+    <InputNumber type="number" max={max} step={selectedType === "Integer" ? 1 : "any"} />
   </Form.Item>
   <Form.Item name="max" label={t("Set maximum value")}>
-    <InputNumber type="number" min={min} />
+    <InputNumber type="number" min={min} step={selectedType === "Integer" ? 1 : "any"} />
   </Form.Item>
 </>
web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/IntegerField/index.tsx (2)

Line range hint 1-53: Consider renaming the component for clarity.

Based on the AI summary, this component is being used for both "Integer" and "Number" field types. The current name "IntegerField" might be misleading since it handles floating-point numbers as well.

Consider renaming to "NumberField" to better reflect its actual usage:

-const IntegerField: React.FC<Props> = ({ multiple, min, max }) => {
+const NumberField: React.FC<Props> = ({ multiple, min, max }) => {

Also update the export:

-export default IntegerField;
+export default NumberField;

Line range hint 6-10: Consider enhancing type safety with discriminated union.

The component props could benefit from a more specific type definition to distinguish between integer and float field configurations.

Consider updating the Props type:

type BaseProps = {
  multiple: boolean;
  min?: number;
  max?: number;
};

type IntegerFieldProps = BaseProps & {
  fieldType: "integer";
};

type FloatFieldProps = BaseProps & {
  fieldType: "float";
  precision?: number;
};

type Props = IntegerFieldProps | FloatFieldProps;
web/src/components/organisms/DataConverters/model.ts (1)

Line range hint 5-53: Consider adding validation for metadata-specific properties.

The fromGraphQLModel function handles both regular fields and metadata fields in a similar way. If MetadataField has additional properties or constraints, consider adding specific validation for them.

Consider refactoring the metadata field mapping into a separate function for better maintainability:

const mapMetadataField = (field: any): MetadataField => ({
  id: field.id,
  description: field.description,
  title: field.title,
  type: field.type,
  key: field.key,
  unique: field.unique,
  isTitle: field.isTitle,
  multiple: field.multiple,
  required: field.required,
  typeProperty: field.typeProperty,
});

Then use it in the main function:

    metadataSchema: {
      id: model.metadataSchema?.id,
-     fields: model.metadataSchema?.fields?.map(
-       field =>
-         ({
-           id: field.id,
-           description: field.description,
-           title: field.title,
-           type: field.type,
-           key: field.key,
-           unique: field.unique,
-           isTitle: field.isTitle,
-           multiple: field.multiple,
-           required: field.required,
-           typeProperty: field.typeProperty,
-         }) as MetadataField,
-     ),
+     fields: model.metadataSchema?.fields?.map(mapMetadataField),
    },
web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx (2)

19-26: Add documentation for min/max property handling.

The min/max computation logic handles both legacy and new properties, but this isn't immediately clear to other developers.

Add JSDoc comments to explain the property precedence:

+ /**
+  * Computes the minimum value using either the legacy 'min' property
+  * or the new 'numberMin' property, with 'min' taking precedence.
+  */
  const min = useMemo(
    () => field?.typeProperty?.min ?? field?.typeProperty?.numberMin,
    [field?.typeProperty?.min, field?.typeProperty?.numberMin],
  );

+ /**
+  * Computes the maximum value using either the legacy 'max' property
+  * or the new 'numberMax' property, with 'max' taking precedence.
+  */
  const max = useMemo(
    () => field?.typeProperty?.max ?? field?.typeProperty?.numberMax,
    [field?.typeProperty?.max, field?.typeProperty?.numberMax],
  );

Line range hint 27-74: Enhance validation logic and error messaging.

The current validation implementation has several areas for improvement:

  1. The min/max violation error message is empty
  2. No explicit validation for float vs integer values
  3. Type checking could be more robust

Consider implementing these improvements:

  rules={[
    {
      required: field.required,
      message: t("Please input field!"),
    },
    {
      validator: (_, value) => {
        const isError = Array.isArray(value) 
-         ? value.some(v => validate(v))
-         : validate(value);
+         ? value.some(v => !isValidNumber(v) || validate(v))
+         : !isValidNumber(value) || validate(value);
        if (isError) {
-         return Promise.reject();
+         return Promise.reject(
+           new Error(t("Value must be between {min} and {max}", { min, max }))
+         );
        }
        return Promise.resolve();
      },
-     message: "",
    },
  ]}

Add a helper function for number validation:

const isValidNumber = (value: unknown): boolean => {
  if (typeof value !== 'number') return false;
  return !Number.isNaN(value) && Number.isFinite(value);
};
web/src/components/molecules/Request/Details/ItemForm.tsx (2)

Line range hint 1-91: Consider enhancing error handling for unknown field types.

While the current implementation works well for known field types, it might be worth adding a fallback for unknown field types to improve robustness.

Consider updating the field rendering logic:

            const FieldComponent = FIELD_TYPE_COMPONENT_MAP[field.type];
+           if (!FieldComponent) {
+             console.warn(`Unknown field type: ${field.type}`);
+             return null;
+           }
            return (
              <div key={field.id}>
                <FieldComponent field={field} disabled />
              </div>
            );

Line range hint 82-91: Consider using theme variables for styling.

The hardcoded color values in the styled component could be replaced with theme variables for better maintainability and consistency.

Consider updating the styles:

  .ant-input-out-of-range input,
  .ant-input-out-of-range textarea {
-    color: #ff4d4f !important;
+    color: ${props => props.theme.colors.error} !important;
  }
web/src/components/molecules/Schema/fieldTypes.ts (1)

55-60: LGTM! Consider documenting numeric type constraints.

The new Number field type is well-structured and consistently implemented. The color scheme matches the Integer type, creating a cohesive visual grouping for numeric fields.

Consider adding a JSDoc comment above the fieldTypes object to document the expected value ranges or precision for numeric types (Integer vs Number).

web/src/components/molecules/Schema/FieldList.tsx (1)

Line range hint 1-180: Consider enhancing field type documentation.

While the code change is correct, it would be helpful to add JSDoc comments for the FieldListItem type to document the relationship between field types and their implementations.

Add documentation like this:

+/**
+ * Represents a category of fields in the schema editor
+ * @property title - The display name of the category
+ * @property fields - Array of field types supported in this category
+ *                   For number fields, supports both Integer and Number (float)
+ */
 type FieldListItem = {
   title: string;
   fields: FieldType[];
 };
web/src/components/molecules/Content/Table/DropdownRender/index.tsx (1)

104-105: Consider adding step prop for float input.

While the current implementation works, consider adding the step prop to InputNumber for better float input control (e.g., step="0.1" or step="0.01").

Example enhancement:

-              <InputNumber onChange={onNumberChange} stringMode placeholder="Enter the value" />
+              <InputNumber 
+                onChange={onNumberChange} 
+                stringMode 
+                step={filter.type === "Number" ? "0.01" : "1"}
+                placeholder="Enter the value" 
+              />
web/e2e/project/item/fields/float.spec.ts (4)

16-48: Consider adding edge cases for float validation.

While the basic float field functionality is well tested, consider adding test cases for:

  • Invalid float inputs (e.g., "abc", special characters)
  • Decimal precision edge cases (e.g., very small/large numbers)
  • Locale-specific decimal separators

Would you like me to provide example test cases for these scenarios?


84-93: Enhance validation boundary testing.

The test verifies that invalid min/max combinations are prevented, but consider:

  1. Adding explicit error message verification
  2. Testing edge cases at the boundaries
  3. Verifying validation message content
// Example additional test cases:
await expect(page.getByText("Maximum value must be greater than minimum value")).toBeVisible();
await page.getByLabel("Set minimum value").fill("2.1");
await page.getByLabel("Set maximum value").fill("2.1");
await expect(page.getByText("Maximum value must be greater than minimum value")).toBeVisible();

99-107: Improve default value validation testing.

The test could be more explicit about why certain default values are invalid. Consider:

  1. Adding comments explaining validation rules
  2. Testing all validation scenarios systematically
  3. Verifying error messages for invalid default values
// Example structured validation:
// Test: Default value must be within min/max range
await page.getByLabel("Set default value").fill("1.0"); // Below min (2.1)
await expect(page.getByRole("button", { name: "OK" })).toBeDisabled();
await expect(page.getByText("Value must be greater than or equal to 2.1")).toBeVisible();

await page.getByLabel("Set default value").fill("11.0"); // Above max (10.1)
await expect(page.getByRole("button", { name: "OK" })).toBeDisabled();
await expect(page.getByText("Value must be less than or equal to 10.1")).toBeVisible();

1-123: Consider extracting common test patterns into helper functions.

The test file is well-structured but could benefit from some architectural improvements:

  1. Extract common field creation/validation patterns into helper functions
  2. Create a shared validation utility for testing numeric constraints
  3. Consider using test data builders for complex field configurations

This would improve maintainability and make it easier to add new test cases.

web/src/components/molecules/Schema/FieldModal/hooks.ts (1)

Line range hint 177-187: Enhance type safety for numeric default values.

While the implementation is generally good, consider strengthening the type checking for float values to ensure data integrity.

Consider this improvement:

  case "Integer":
  case "Number": {
    const defaultValue = Array.isArray(values.defaultValue)
-     ? values.defaultValue.filter((value: number | string) => typeof value === "number")
+     ? values.defaultValue.filter((value: number | string) => {
+         if (values.type === "Integer") {
+           return typeof value === "number" && Number.isInteger(value);
+         }
+         return typeof value === "number" && Number.isFinite(value);
+       })
      : (values.defaultValue ?? "");
    return {
      [values.type === "Integer" ? "integer" : "number"]: {
        defaultValue,
        min: values.min ?? null,
        max: values.max ?? null,
      },
    };
  }
web/src/i18n/translations/en.yml (2)

355-356: Consider using more familiar terminology for float field type

The term "Fractional" might not be immediately clear to all users. Consider using more commonly recognized terms like:

  • "Decimal Number" (most user-friendly)
  • "Float" (technical but widely used)
  • "Floating Point Number" (most precise)
-Float: ''
-Fractional: ''
+Float: ''
+Decimal Number: ''

462-559: Consider adding float-specific validation error messages

While general success/error messages are present, consider adding specific validation error messages for float fields, such as:

  • Invalid decimal format
  • Value exceeds minimum/maximum limits
+Invalid decimal format.: ''
+Value must be between {{min}} and {{max}}.: ''
web/src/i18n/translations/ja.yml (1)

68-68: Consider translating common technical terms

Some common technical terms are left in English. While this is sometimes acceptable, consider adding Japanese translations for better user experience.

-ID: ''
+ID: 'ID'

-Webhook: ''
+Webhook: 'ウェブフック'

-example: ''
+example: '例'

Also applies to: 224-224, 395-395

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between af0bd0d and 38c2b40.

⛔ Files ignored due to path filters (1)
  • web/src/components/atoms/Icon/Icons/infinity.svg is excluded by !**/*.svg
📒 Files selected for processing (24)
  • web/e2e/project/item/fields/float.spec.ts (1 hunks)
  • web/src/components/atoms/Icon/icons.ts (2 hunks)
  • web/src/components/atoms/InputNumber/index.tsx (1 hunks)
  • web/src/components/molecules/Common/Form/GroupItem/index.tsx (4 hunks)
  • web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx (2 hunks)
  • web/src/components/molecules/Content/Form/fields/FieldComponents/index.ts (0 hunks)
  • web/src/components/molecules/Content/Form/fields/FieldTypesMap.ts (1 hunks)
  • web/src/components/molecules/Content/Form/index.tsx (2 hunks)
  • web/src/components/molecules/Content/Table/DropdownRender/hooks.ts (2 hunks)
  • web/src/components/molecules/Content/Table/DropdownRender/index.tsx (1 hunks)
  • web/src/components/molecules/Request/Details/ItemForm.tsx (1 hunks)
  • web/src/components/molecules/Schema/FieldList.tsx (1 hunks)
  • web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/IntegerField/index.tsx (1 hunks)
  • web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/index.tsx (1 hunks)
  • web/src/components/molecules/Schema/FieldModal/FieldValidationInputs/index.tsx (1 hunks)
  • web/src/components/molecules/Schema/FieldModal/hooks.ts (3 hunks)
  • web/src/components/molecules/Schema/fieldTypes.ts (2 hunks)
  • web/src/components/molecules/Schema/types.ts (5 hunks)
  • web/src/components/organisms/DataConverters/model.ts (2 hunks)
  • web/src/components/organisms/DataConverters/schema.ts (2 hunks)
  • web/src/gql/graphql-client-api.tsx (2 hunks)
  • web/src/gql/queries/model.ts (1 hunks)
  • web/src/i18n/translations/en.yml (1 hunks)
  • web/src/i18n/translations/ja.yml (1 hunks)
💤 Files with no reviewable changes (1)
  • web/src/components/molecules/Content/Form/fields/FieldComponents/index.ts
🔇 Additional comments (36)
web/src/components/molecules/Content/Form/fields/FieldTypesMap.ts (3)

1-11: LGTM! Clean import organization.

The imports are well-organized and the switch to NumberField aligns with the unified number handling approach.


16-16: LGTM! Standardization with DefaultField.

The addition of Text: DefaultField mapping indicates a good move towards standardization of basic field types.


24-25: Verify number validation handling for Integer vs Float types.

While using NumberField for both Integer and Number types promotes code reuse, we should ensure proper type-specific validation is in place.

Let's check the NumberField implementation:

Consider documenting the validation behavior differences between Integer and Number types in the component's documentation or comments, if not already present.

✅ Verification successful

Based on the code analysis, I can now provide a final response:

Integer and Number field validation is properly handled with type-specific constraints.

The implementation shows proper type-specific validation:

  • Integer fields use SchemaFieldInteger with min and max as int values
  • Number fields use SchemaFieldNumber with min and max as float64 values
  • The NumberField component handles validation through:
    • Type-specific min/max constraints from field properties
    • Input validation using InputNumber component
    • Form validation rules that check value ranges
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify NumberField handles both integer and float validation correctly

# Check NumberField implementation for type-specific validation
ast-grep --pattern 'const NumberField = $_' -A 20

Length of output: 51


Script:

#!/bin/bash
# Let's try different approaches to locate NumberField implementation

# Search for NumberField in the codebase
rg "NumberField" -A 10

# Also search for potential file names containing NumberField
fd "NumberField"

# Search for number validation related patterns
rg "validateNumber|numberValidation|isInteger|isFloat" -A 5

Length of output: 52164


Script:

#!/bin/bash
# Let's examine the NumberField implementation
cat web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx

# Also search for any type-specific validation in the codebase
rg "type.*Integer|type.*Number" -A 5

Length of output: 64014

web/src/components/organisms/DataConverters/schema.ts (2)

1-1: LGTM: Import statement updated for better type safety.

The change from Field to GroupField aligns with the broader type system improvements.


30-30: Verify type casting safety.

The type casting to GroupField might be too restrictive. Please ensure that all possible field types from the GraphQL schema can be safely cast to GroupField.

web/src/components/molecules/Schema/FieldModal/FieldValidationInputs/index.tsx (1)

Line range hint 23-31: LGTM! Clean implementation of float field support.

The changes correctly extend the validation inputs to support both Integer and Number types while maintaining the existing behavior for other field types.

web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/IntegerField/index.tsx (1)

20-22: LGTM! Improved validation logic for min/max checks.

The validation now correctly handles edge cases by explicitly checking the type of min and max before comparison. This fixes potential issues when min/max values are set to 0.

web/src/components/organisms/DataConverters/model.ts (2)

2-2: LGTM! Import statement updated for better type safety.

The addition of MetadataField type import aligns with the separation of concerns between regular fields and metadata fields.


49-49: Verify type compatibility between Field and MetadataField.

The change from Field to MetadataField type assertion suggests these types might have different structures. Let's ensure they are compatible to prevent runtime issues.

✅ Verification successful

Based on the shell script results, I can now generate the final response since I have clear information about both types:

Type assertion from Field to MetadataField is safe and correct

The type assertion is valid because MetadataField is defined as an intersection type that extends Field with a more restrictive type property, specifically limiting it to certain field types: "Tag", "Bool", "Checkbox", "Date", "Text", or "URL". This means every MetadataField is a Field with additional constraints, making the type assertion safe.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the type definitions to ensure compatibility

# Test 1: Find the type definitions
echo "=== Searching for Field and MetadataField type definitions ==="
ast-grep --pattern 'type Field = {
  $$$
}'

ast-grep --pattern 'type MetadataField = {
  $$$
}'

# Test 2: Check for any specific metadata-related properties
echo "=== Checking for metadata-specific properties ==="
rg -A 5 'type MetadataField'

Length of output: 1675

web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx (2)

Line range hint 17-75: Verify consistent usage of NumberField component.

The component rename from IntegerField to NumberField requires verification of all usage locations to ensure proper migration.

✅ Verification successful

Based on the search results, I can now provide a final response:

NumberField component implementation is properly integrated

The verification shows that:

  • The NumberField component is correctly mapped for both Integer and Number field types in FieldTypesMap.ts
  • There are remaining IntegerField references, but they are in a different context (FieldModal/FieldDefaultInputs) and not related to the form field component
  • The component is properly supported by the GraphQL schema with corresponding types and conditions
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining references to IntegerField
echo "Checking for remaining IntegerField references..."
rg "IntegerField"

# Search for NumberField usage to verify proper implementation
echo "Checking NumberField usage patterns..."
rg "NumberField"

Length of output: 15968


17-17: 🛠️ Refactor suggestion

Consider enhancing type safety for numeric values.

While the component rename from IntegerField to NumberField is appropriate, consider making the numeric types more explicit in the props interface.

type DefaultFieldProps = {
  field: Field;
  itemGroupId?: string;
  disabled?: boolean;
+ value?: number | number[];  // Add explicit type for the field value
};

Likely invalid or redundant comment.

web/src/components/molecules/Request/Details/ItemForm.tsx (1)

Line range hint 58-63: LGTM! Clean and type-safe field rendering.

The simplified field rendering logic using FIELD_TYPE_COMPONENT_MAP is a good improvement. It provides better maintainability and makes it easier to add new field types like the float field.

Let's verify that the field type mapping includes all necessary types:

✅ Verification successful

LGTM! Field type mapping is complete and includes float field support

Looking at the FIELD_TYPE_COMPONENT_MAP definition in web/src/components/molecules/Content/Form/fields/FieldTypesMap.ts, we can see that float fields are properly handled through the NumberField component, which is mapped to both Integer and Number field types. The simplified field rendering logic in ItemForm.tsx correctly uses this mapping.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if FIELD_TYPE_COMPONENT_MAP includes all field types used in the codebase
# Expected: Should find the mapping definition with float field support

rg -A 10 "FIELD_TYPE_COMPONENT_MAP.*=.*{" 

Length of output: 167


Script:

#!/bin/bash
# Let's try a simpler search pattern to find the FIELD_TYPE_COMPONENT_MAP definition
rg "FIELD_TYPE_COMPONENT_MAP" -A 10

Length of output: 8225


Script:

#!/bin/bash
# Let's also search for field type definitions to cross-reference
rg "type.*Field.*=.*{" -A 5

Length of output: 127


Script:

#!/bin/bash
# Search for field type enum or constants
ast-grep --pattern 'export const FIELD_TYPE = {
  $$$
}'

Length of output: 58


Script:

#!/bin/bash
# Let's try a different pattern to find field type definitions
rg "export.*type.*Field" -A 2

# And let's check the imports in FieldTypesMap to see all field components
rg "import.*Field.*from" web/src/components/molecules/Content/Form/fields/FieldTypesMap.ts -A 15

Length of output: 65026

web/src/components/molecules/Schema/fieldTypes.ts (1)

55-60: Verify translation usage and field type consistency.

Let's ensure the new field type and translations are consistently used across the UI components.

Also applies to: 132-134

✅ Verification successful

Based on the search results, I can see that:

  1. The field type "Float" and its description "Fractional" are properly defined in fieldTypes.ts
  2. The translations are correctly used with the t() function in the same file
  3. The field type "Number" is consistently used across the codebase for handling numeric values, including:
    • Form components (NumberField)
    • Field type definitions
    • Type conditions and operators

Let me verify the field type implementation to ensure complete consistency:


Let me try one more search to verify the field type definition:


Based on the gathered evidence, I can now provide a final response:

Field type "Number" is properly integrated and consistent

The implementation is verified to be correct and consistent across the codebase:

  • Field type is properly defined with translations for "Float" and "Fractional"
  • NumberField component is fully implemented with proper validation and input handling
  • Type is consistently handled in form components, dropdown renderers, and field modal
  • Integration points (case statements) properly handle the "Number" type across components
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for consistent usage of field type name
echo "Checking field type naming consistency..."
rg -g '*.{ts,tsx}' "(?i)(float|number|decimal)" web/src/components/

# Check for translation key usage
echo "Checking translation key usage..."
rg -g '*.{ts,tsx}' "t\([\"']Float[\"']\)|t\([\"']Fractional[\"']\)" web/src/components/

Length of output: 42229


Script:

#!/bin/bash
# Check NumberField implementation
ast-grep --pattern 'const NumberField: React.FC<$_> = $_'

# Check field type usage in schema
rg -g '*.{ts,tsx}' '"Number".*:.*{' web/src/components/

Length of output: 6647


Script:

#!/bin/bash
# Check field type mapping and usage
rg -g '*.{ts,tsx}' '"Number":\s*{' web/src/components/

# Check field type handling in form components
rg -g '*.{ts,tsx}' 'case\s+"Number"' web/src/components/

Length of output: 469

web/src/components/molecules/Schema/types.ts (5)

5-5: LGTM! Improved type safety for metadata fields.

The change from Field[] to MetadataField[] properly restricts metadata fields to supported types, preventing invalid field types from being used in metadata.


13-13: LGTM! Well-structured type definition for group schemas.

The GroupSchema type properly extends Schema while restricting field types, preventing nested groups and improving type safety.


29-33: LGTM! Well-designed type constraints for specialized fields.

The type definitions effectively use TypeScript's type system to enforce valid field types:

  • GroupField prevents nested groups through Exclude
  • MetadataField precisely specifies allowed types through Extract

142-142: LGTM! Consistent use of GroupSchema type.

The schema property now correctly uses the more specific GroupSchema type, maintaining type safety throughout the group hierarchy.


46-46: Verify the distinction between "Integer" and "Number" types.

While adding the "Number" type aligns with the float field feature, ensure that:

  1. The distinction between "Integer" and "Number" is clearly documented
  2. UI components handle both types appropriately
  3. Validation logic correctly enforces type constraints
web/src/components/molecules/Content/Table/DropdownRender/index.tsx (1)

104-105: Verify float precision handling in InputNumber component.

The condition now handles both "Integer" and "Number" types, which is good for supporting float fields. The use of stringMode in InputNumber is crucial for maintaining float precision.

Let's verify the InputNumber component's configuration for float handling:

✅ Verification successful

Let me gather more information about the InputNumber component and its float handling capabilities.


Let me check the InputNumber component implementation since we found it's a wrapper around AntD's InputNumber.


Based on the code inspection, I can now provide the final response:

Float precision handling is properly implemented with InputNumber component

The implementation is correct and safe for float handling because:

  • The component uses AntD's InputNumber with stringMode which prevents floating-point precision issues
  • The custom InputNumber wrapper properly handles type generics <T extends string | number> supporting both integer and float values
  • Proper validation is implemented for min/max bounds without losing precision
  • The NumberField component correctly handles numeric validation and type properties
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check InputNumber component's props and usage
# Expected: Find any precision or float-specific configurations

# Search for InputNumber component definition and usage
ast-grep --pattern 'const InputNumber = $$$' -A 10
rg -g '*.tsx' -g '*.ts' 'InputNumber.*stringMode' -A 3

Length of output: 587


Script:

#!/bin/bash
# Search for InputNumber imports and any custom configurations
rg -g '*.tsx' -g '*.ts' '^import.*InputNumber' -B 1 -A 1

# Search for any float-related configurations or tests
rg -g '*.tsx' -g '*.ts' -g '*.test.*' 'InputNumber.*float|InputNumber.*decimal|InputNumber.*precision' -A 3

# Look for any numeric type definitions or interfaces
ast-grep --pattern 'type $_ = {
  $$$
  type: "Number" | "Integer"
  $$$
}'

# Check for any numeric value processing or validation
rg -g '*.tsx' -g '*.ts' 'onNumberChange.*float|onNumberChange.*number' -A 5

Length of output: 2262


Script:

#!/bin/bash
# Check the InputNumber component implementation
cat web/src/components/atoms/InputNumber/index.tsx

# Check for number field handling in related components
cat web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx

# Look for any number-related type definitions
rg -g '*.ts' -g '*.tsx' 'type.*Number.*=|interface.*Number.*{' -A 3

Length of output: 3272

web/src/gql/queries/model.ts (1)

110-114: LGTM! The SchemaFieldNumber fragment is well-structured.

The implementation follows the established patterns in the codebase:

  • Consistent naming with other numeric fields
  • Clear aliasing for min/max fields
  • Proper placement within the typeProperty union

Let's verify the GraphQL schema compatibility:

✅ Verification successful

The SchemaFieldNumber fragment is correctly aligned with the schema definition

The verification confirms that:

  • The type exists in the GraphQL schema (server/schemas/field.graphql)
  • The field types match exactly:
    • defaultValue: Any
    • min: Float
    • max: Float
  • The type is properly generated in both client and server code with consistent field definitions
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the SchemaFieldNumber type exists in the schema
# and has the expected fields

# Test: Search for SchemaFieldNumber type definition
# Expect: Type definition with fields defaultValue, min, and max
rg -A 5 "type SchemaFieldNumber"

Length of output: 2418

web/src/components/atoms/Icon/icons.ts (2)

71-71: LGTM! Import follows established patterns.

The import statement follows the existing conventions and maintains alphabetical ordering.


130-130: LGTM! Export mapping is consistent with codebase patterns.

The infinity icon export follows the established naming conventions and maintains alphabetical ordering. This addition appropriately supports the float field feature by providing an icon for representing infinite/unlimited values.

web/e2e/project/item/fields/float.spec.ts (1)

1-14: LGTM: Well-structured test setup and teardown.

The test hooks ensure proper isolation and cleanup for each test case.

web/src/components/molecules/Common/Form/GroupItem/index.tsx (3)

15-15: LGTM! Import addition improves type safety

The addition of GroupField import aligns with the state type change and provides better type specificity.


119-119: Verify GroupField type compatibility

The state type change from Field[] to GroupField[] improves type specificity. However, we should verify that this change doesn't break any existing functionality.

✅ Verification successful

Based on the search results, I can now provide a final response:

Type change from Field[] to GroupField[] is safe and intentional

The type change is correct and well-defined in the codebase:

  • GroupField is defined in web/src/components/molecules/Schema/types.ts as type GroupField = Field & { type: Exclude<FieldType, "Group"> }
  • This type ensures that group fields cannot contain nested groups, which is a valid constraint
  • No other components using this state are affected as they would receive the more specific type
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check usage of GroupField type and potential breaking changes

# Test 1: Check for any direct Field[] type usage that might need updating
rg "useState<Field\[\]>" -A 2

# Test 2: Check GroupField type definition and usage
ast-grep --pattern 'type GroupField = $_'

Length of output: 1736


Script:

#!/bin/bash
# Let's try a different approach to find Field and GroupField type definitions and usages

# Search for Field and GroupField type definitions
rg "type (Field|GroupField)" -A 3

# Search for Field[] and GroupField[] usage in useState
rg "useState.*<(Field|GroupField)\[\]>"

# Search for imports of Field or GroupField types
rg "import.*\{.*(?:Field|GroupField).*\}"

# Look for files that might contain field-related type definitions
fd "types?.ts" --exec rg "Field" {}

Length of output: 46257


235-241: Document width handling and verify field type mapping

The simplified rendering logic using FIELD_TYPE_COMPONENT_MAP improves maintainability. However, consider:

  1. Adding a comment explaining the width handling for geometry fields
  2. Ensuring all field types are properly mapped

Consider adding a comment above the StyledFormItemWrapper:

// Special width handling for geometry fields to accommodate their complex UI requirements
✅ Verification successful

Based on the results from our previous scripts, we can now verify the field type mapping. We found that all field types defined in FieldType (web/src/components/molecules/Schema/types.ts) are properly mapped in the FIELD_TYPE_COMPONENT_MAP that we saw earlier, including:

  • Basic types: Text, TextArea, Date, Bool, Select, Tag, Integer, Number, Checkbox, URL
  • Complex types: MarkdownText, Asset, Reference, Group
  • Geometry types: GeometryObject, GeometryEditor

The width handling for geometry fields is also appropriate given their complex UI requirements.

Field type mapping is complete and width handling is appropriate

The special width handling for geometry fields is justified by their complex UI requirements, and all field types from the schema definition are properly mapped to their corresponding components. No documentation is needed as the code is self-explanatory.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify field type mapping completeness

# Test 1: Find all defined field types
rg "type.*Field.*=.*{.*type.*:.*string" -A 5

# Test 2: Check FIELD_TYPE_COMPONENT_MAP implementation
ast-grep --pattern 'const FIELD_TYPE_COMPONENT_MAP = {
  $$$
}'

Length of output: 1736


Script:

#!/bin/bash
# Let's try to find field type definitions with a different pattern
rg "type.*=.*'[A-Za-z]+'" -A 1

# Also search for any field type references in the codebase
rg "field\.type\s*===?\s*['\"]([A-Za-z]+)['\"]" -g "*.{ts,tsx}"

# And look for any type definitions that might define field types
ast-grep --pattern 'type Field = {
  $$$
}'

Length of output: 94214


Script:

#!/bin/bash
# Let's find the field types defined in the codebase and compare with the component map
rg "type\s+FieldType\s*=\s*" -A 20

# Also check the implementation of FIELD_TYPE_COMPONENT_MAP
rg "FIELD_TYPE_COMPONENT_MAP\s*=\s*{" -A 20

Length of output: 2919

web/src/components/molecules/Schema/FieldModal/hooks.ts (2)

140-141: LGTM: Robust fallback handling for numeric constraints.

The fallback pattern using the nullish coalescing operator (??) appropriately handles both integer and float field types by checking both legacy and new number properties.


Line range hint 1-399: Verify float field implementation completeness.

Let's verify that all necessary components are in place to support float fields.

✅ Verification successful

Based on the verification results, I can now provide a final response:

Number field implementation is complete and properly integrated

The implementation shows:

  • Proper type definitions for Number fields in the schema
  • Complete NumberField component with validation for min/max values
  • Correct field type mapping for both Integer and Number fields to the NumberField component
  • Support for multiple values and proper form validation
  • Consistent handling of number properties including min/max constraints
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify float field implementation completeness

# Test 1: Check for float field type definition
echo "Checking for float field type definition..."
rg -l "type FieldType.*=.*'Number'" 

# Test 2: Check for float field validation
echo "Checking for float field validation..."
rg -l "validateFloat|validateNumber" 

# Test 3: Check for float field components
echo "Checking for float field components..."
rg -l "NumberField|FloatField" 

# Test 4: Check for float field tests
echo "Checking for float field tests..."
rg -l "test.*float.*field" --glob "**/*.test.*"

Length of output: 1077


Script:

#!/bin/bash
# Let's check the implementation details more thoroughly

# Check for Number field type definition and handling
echo "=== Checking Number field type definitions ==="
rg "type.*FieldType.*=.*" -A 5

# Check NumberField component implementation
echo -e "\n=== Checking NumberField component implementation ==="
cat "web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx"

# Check field type mapping
echo -e "\n=== Checking field type mapping ==="
cat "web/src/components/molecules/Content/Form/fields/FieldTypesMap.ts"

# Check type property handling for Number fields
echo -e "\n=== Checking type property handling ==="
rg "case 'Number':" -A 10 -B 2

Length of output: 6656

web/src/components/molecules/Content/Table/DropdownRender/hooks.ts (1)

139-139: LGTM: Unified numeric type handling

The addition of "Number" case alongside "Integer" provides a clean approach to handle both integer and floating-point values with consistent operators.

web/src/i18n/translations/en.yml (3)

290-299: LGTM! Field types are well organized

The float field type is properly categorized with other numeric types, maintaining a logical and consistent organization.


333-334: LGTM! Validation messages are clear

The minimum and maximum value validation messages are clear and consistent with other field types.


1-559: Verify translation completeness across language files

Let's ensure all float-related translations are consistently present across language files.

✅ Verification successful

Let me gather more information to verify the presence and consistency of all float-related translations.


All float-related translations are present and consistent

The verification shows that all float-related translations and their associated validation strings are properly present in both English (en.yml) and Japanese (ja.yml) translation files. This includes:

  • Float/Fractional field types
  • Number field type
  • Integer/Int field types
  • Validation strings for minimum and maximum values
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if float-related translations exist in all language files
# Expected: All language files should contain the same float-related keys

# Find all translation files
echo "Checking translation files for float-related keys..."
fd -e yml -e yaml . web/src/i18n/translations/ -x rg -l "Float:|Fractional:|Set minimum value:|Set maximum value:"

# Compare float-related keys across files
echo "Comparing float-related translations across files..."
rg -A 1 "Float:" web/src/i18n/translations/

Length of output: 641


Script:

#!/bin/bash
# Check for all validation-related translations (min/max values) across files
echo "Checking validation-related translations..."
rg "Set (minimum|maximum) value:" web/src/i18n/translations/

# Check if there are any other number-related fields that might need float validation
echo -e "\nChecking for number-related fields..."
rg "Number:|Int:|Integer:" web/src/i18n/translations/

Length of output: 788

web/src/i18n/translations/ja.yml (1)

1-559: Overall translation quality is good

The translations are generally well-done and maintain consistency in terminology. The language is natural and appropriate for a Japanese business application.

web/src/components/molecules/Content/Form/index.tsx (2)

627-627: LGTM! Clean implementation of field type handling.

The implementation consistently uses FIELD_TYPE_COMPONENT_MAP for both regular and meta fields, which provides a clean and maintainable way to support new field types, including the float field.

Also applies to: 643-643


Line range hint 643-647: Verify field type handling in meta fields.

The meta field rendering uses the same FIELD_TYPE_COMPONENT_MAP as regular fields. Let's verify that all field types, including the new float field type, are properly supported in meta fields.

✅ Verification successful

Let me gather more information about the field type component mapping to ensure all field types are properly supported.


Based on the gathered information, I can now generate the final response:

All field types are properly supported in meta fields

Comparing the field types from web/src/components/molecules/Schema/types.ts with the component mappings in web/src/components/molecules/Content/Form/fields/FieldTypesMap.ts, all field types including "Number" are properly supported:

  • The FIELD_TYPE_COMPONENT_MAP includes all necessary mappings:
    • Text -> DefaultField
    • Number -> NumberField
    • Integer -> NumberField
    • TextArea -> TextareaField
    • MarkdownText -> MarkdownField
    • Date -> DateField
    • Bool -> BoolField
    • Select -> SelectField
    • Tag -> TagField
    • Checkbox -> CheckboxField
    • URL -> URLField
    • GeometryObject/GeometryEditor -> GeometryField
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if all field types used in meta fields are supported in FIELD_TYPE_COMPONENT_MAP

# Search for field type definitions and their usage in meta fields
echo "Searching for field type definitions..."
ast-grep --pattern 'type FieldType = $$$'

echo -e "\nSearching for meta field type usage..."
rg -A 2 'metadataSchema.*fields.*type'

Length of output: 6551


Script:

#!/bin/bash
# Search for FIELD_TYPE_COMPONENT_MAP definition and its usage
echo "Searching for FIELD_TYPE_COMPONENT_MAP definition..."
ast-grep --pattern 'const FIELD_TYPE_COMPONENT_MAP = {
  $$$
}'

echo -e "\nSearching for field components imports..."
rg -A 5 'import.*Field.*from'

Length of output: 45784

web/src/gql/graphql-client-api.tsx (1)

5298-5302: Ensure all usages of GetModelQuery accommodate the updated type.

The GetModelQuery type now includes numberMin and numberMax properties within SchemaFieldNumber. Please verify that all components and functions using GetModelQuery are updated to handle these new properties appropriately to prevent potential type inconsistencies or runtime errors.

You can run the following script to locate all usages of GetModelQuery in the codebase:

web/src/components/molecules/Schema/types.ts Show resolved Hide resolved
web/src/i18n/translations/ja.yml Outdated Show resolved Hide resolved
web/src/i18n/translations/ja.yml Outdated Show resolved Hide resolved
web/src/i18n/translations/ja.yml Outdated Show resolved Hide resolved
web/src/i18n/translations/ja.yml Outdated Show resolved Hide resolved
@github-actions github-actions bot added the web label Oct 29, 2024
@caichi-t caichi-t force-pushed the fix-web/float-field branch from beb7d05 to 9bf35f6 Compare October 29, 2024 08:00
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (4)
web/src/components/molecules/Member/MemberTable/index.tsx (2)

250-252: Consider adding ARIA label for better accessibility.

The delete button implementation is correct, but could benefit from accessibility improvements.

- <DeleteButton onClick={() => handleMemberDelete(props.selectedRows)}>
+ <DeleteButton 
+   onClick={() => handleMemberDelete(props.selectedRows)}
+   aria-label={t("Remove selected members")}>

297-325: LGTM: Well-structured styled components with a minor suggestion.

The styled components are well-organized with proper overflow handling. Consider using theme variables for colors and spacing for better maintainability.

- color: #00000073;
+ color: ${props => props.theme.colors.textSecondary};

- gap: 8px;
+ gap: ${props => props.theme.spacing.sm};
web/src/i18n/translations/en.yml (2)

358-359: Consider revising the Float field description

The term "Fractional" might not be the most intuitive description for float fields. Consider using more common terms like "Decimal numbers" or "Numbers with decimal points" to better convey the purpose to users.

-Float: ''
-Fractional: ''
+Float: ''
+Decimal numbers: ''

336-337: Add float-specific validation messages

Consider adding validation messages specific to float fields, such as decimal precision limits or decimal place restrictions.

 Set minimum value: ''
 Set maximum value: ''
+Set decimal precision: ''
+Set decimal places: ''
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 38c2b40 and 9bf35f6.

📒 Files selected for processing (5)
  • web/src/components/atoms/Icon/icons.ts (4 hunks)
  • web/src/components/molecules/Member/MemberTable/index.tsx (6 hunks)
  • web/src/components/molecules/Workspace/types.ts (2 hunks)
  • web/src/i18n/translations/en.yml (1 hunks)
  • web/src/i18n/translations/ja.yml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/src/components/atoms/Icon/icons.ts
  • web/src/i18n/translations/ja.yml
🔇 Additional comments (7)
web/src/components/molecules/Workspace/types.ts (2)

3-3: LGTM! Import statement follows best practices.

The import of the User type promotes code reuse and maintains DRY principles.


19-19: LGTM! Verify consistent usage of User type.

The change to use the imported User type improves type consistency. Let's verify that this type is used consistently across the codebase.

✅ Verification successful

Let me try a different search pattern to check for any inconsistent User type definitions:


Based on the search results, I can now generate the final response:

The User type is consistently imported and used across the codebase

The imported User type from @reearth-cms/components/molecules/Member/types is used consistently in the workspace-related components. The type is properly defined and referenced in related components like MemberTable, MemberAddModal, and Request components. No inconsistent or duplicate User type definitions were found.

Key observations:

  • The User type is centrally defined in Member/types.ts
  • All workspace-related components correctly import and use this type
  • No conflicting or redundant User type definitions exist in the codebase
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of User type across the codebase
# Expected: All user-related type definitions should reference the imported User type

# Search for any remaining inline user type definitions that could be replaced
rg -g '*.ts' -g '*.tsx' 'type.*User.*=.*{.*name:.*string'

# Search for imports of the User type to verify adoption
rg -g '*.ts' -g '*.tsx' "import.*User.*from.*Member/types"

Length of output: 884


Script:

#!/bin/bash
# Search for type definitions containing "User" and "name" property
ast-grep --pattern 'type User = { $$$name$$$: $_$$$}'

# Search for interface definitions containing "User" and "name" property
ast-grep --pattern 'interface User { $$$name$$$: $_$$$ }'

# Search for all User type references to ensure consistent usage
rg -g '*.ts' -g '*.tsx' 'User(\s|[^\w]|$)' -B1 -A1

Length of output: 105700

web/src/components/molecules/Member/MemberTable/index.tsx (3)

14-14: LGTM: Import of User type.

The addition of the User type import aligns with the enhanced type safety in member handling.


173-204: LGTM: Clean implementation of action buttons.

The conditional rendering of leave/remove actions with proper dividers and dependency management is well implemented.


65-96: Verify the integration with workspace member management.

Let's ensure the new member deletion flow is properly integrated with the workspace management system.

Also applies to: 250-252

✅ Verification successful

Member removal implementation is properly integrated and error-handled

The verification shows that:

  • The member removal functionality is properly integrated with the workspace management system
  • Error handling is implemented with appropriate user notifications
  • The User type is consistently defined and used across components
  • The mutation is properly typed and implemented with Apollo client
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any other components that might need updates for the new User object handling
echo "Checking for components that might need updates..."
rg -l "handleMemberRemoveFromWorkspace" --type typescript

# Check for consistent error handling patterns
echo "Checking error handling patterns in member management..."
rg "handleMemberRemoveFromWorkspace.*catch" --type typescript

Length of output: 425


Script:

#!/bin/bash
# Let's try without typescript file type specification
echo "Checking for components using handleMemberRemoveFromWorkspace..."
rg -l "handleMemberRemoveFromWorkspace"

# Check the implementation and error handling
echo "Checking implementation and error handling patterns..."
rg "handleMemberRemoveFromWorkspace" -A 5

# Look for related workspace member management code
echo "Checking for related workspace member management code..."
rg "MemberTable|WorkspaceMember" -l

Length of output: 5408


Script:

#!/bin/bash
# Let's check the implementation of the mutation and error handling
echo "Checking the mutation implementation..."
rg "RemoveMultipleMembersFromWorkspaceMutation" -A 10

# Check for any error notifications or message handling
echo "Checking error handling and notifications..."
rg "notification.*error|message.*error" web/src/components/organisms/Settings/Members/hooks.ts

# Verify the User type definition
echo "Checking User type definition..."
ast-grep --pattern 'type User = {
  $$$
}'

Length of output: 13574

web/src/i18n/translations/en.yml (2)

1-562: General translation structure looks good

The translations follow consistent YAML formatting with proper indentation and empty string values.


438-562: Error and success messages are well-structured

The notification messages follow a consistent pattern and cover all necessary operations, including the new float field functionality.

web/src/components/molecules/Member/MemberTable/index.tsx Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/NumberField/index.tsx (1)

14-14: LGTM! Consider enhancing type safety for numeric constraints.

The component rename from IntegerField to NumberField better reflects its generic numeric handling capability.

Consider adding runtime type checks for min and max props to ensure they're valid numbers when provided:

const NumberField: React.FC<Props> = ({ multiple, min, max }) => {
+ React.useEffect(() => {
+   if (min !== undefined && !Number.isFinite(min)) {
+     console.warn('NumberField: min prop should be a finite number');
+   }
+   if (max !== undefined && !Number.isFinite(max)) {
+     console.warn('NumberField: max prop should be a finite number');
+   }
+ }, [min, max]);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 9bf35f6 and 5ab70b4.

📒 Files selected for processing (2)
  • web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/NumberField/index.tsx (2 hunks)
  • web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/index.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/index.tsx
🔇 Additional comments (2)
web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/NumberField/index.tsx (2)

55-55: LGTM! Export statement is correct.

The default export follows React component conventions.


Line range hint 14-55: Verify component imports across the codebase.

Since the component has been renamed from IntegerField to NumberField, let's ensure all imports have been updated accordingly.

✅ Verification successful

No remaining references to IntegerField found in the codebase

The search results show that all references are now using NumberField. The component is properly imported and used in:

  • web/src/components/molecules/Schema/FieldModal/FieldDefaultInputs/index.tsx
  • web/src/components/molecules/Content/Form/fields/FieldTypesMap.ts (mapped to both Integer and Number types)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining references to IntegerField
# that might have been missed during the rename

echo "Checking for any remaining IntegerField references..."
rg -l "IntegerField" --type ts --type tsx

echo "Verifying NumberField imports..."
rg -l "NumberField" --type ts --type tsx

Length of output: 330


Script:

#!/bin/bash
# Let's try without specifying file types and use a more general search
echo "Checking for any remaining IntegerField references..."
rg "IntegerField"

echo -e "\nVerifying NumberField imports and usage..."
rg "NumberField"

Length of output: 15983

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (3)
web/src/i18n/translations/en.yml (2)

376-377: Consider improving field type descriptions for clarity

While the additions for float field support are good, the descriptions could be more descriptive and consistent with other numeric field types.

Consider these improvements:

-Float: ''
-Fractional: ''
+Float: 'Decimal numbers'
+Fractional: 'Numbers with decimal points'

This would:

  • Provide clearer descriptions of what these fields accept
  • Match the descriptive pattern used for other field types
  • Help users better understand the difference between integer and float fields

Line range hint 1-1: Add float-specific validation messages

To fully support float fields, consider adding translations for float-specific validation messages.

Add these translations:

+Invalid decimal format: 'Please enter a valid decimal number'
+Decimal places: 'Maximum number of decimal places'
+Set decimal precision: 'Set the number of decimal places allowed'

This would:

  • Provide better user feedback for float field validation
  • Support proper error handling for decimal input
  • Allow configuration of decimal precision
web/src/gql/graphql-client-api.tsx (1)

5322-5326: LGTM: Number field range constraints added correctly

The addition of numberMin and numberMax fields to the SchemaFieldNumber type properly enables float field range validation. This aligns with the PR objective of adding float field support.

Consider adding JSDoc comments to document the purpose and usage of these fields, for example:

            ... on SchemaFieldNumber {
+             /** Default value for the number field */
              defaultValue
+             /** Minimum allowed value (inclusive) */
              numberMin: min
+             /** Maximum allowed value (inclusive) */
              numberMax: max
            }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 5ab70b4 and 875125b.

📒 Files selected for processing (10)
  • web/src/components/atoms/Icon/icons.ts (2 hunks)
  • web/src/components/molecules/Common/Form/GroupItem/index.tsx (4 hunks)
  • web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx (2 hunks)
  • web/src/components/molecules/Content/Form/index.tsx (2 hunks)
  • web/src/components/molecules/Schema/FieldList.tsx (1 hunks)
  • web/src/components/molecules/Schema/FieldModal/hooks.ts (2 hunks)
  • web/src/components/molecules/Schema/types.ts (5 hunks)
  • web/src/gql/graphql-client-api.tsx (2 hunks)
  • web/src/i18n/translations/en.yml (1 hunks)
  • web/src/i18n/translations/ja.yml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
  • web/src/components/atoms/Icon/icons.ts
  • web/src/components/molecules/Common/Form/GroupItem/index.tsx
  • web/src/components/molecules/Content/Form/fields/FieldComponents/NumberField.tsx
  • web/src/components/molecules/Content/Form/index.tsx
  • web/src/components/molecules/Schema/FieldList.tsx
  • web/src/components/molecules/Schema/FieldModal/hooks.ts
  • web/src/components/molecules/Schema/types.ts
  • web/src/i18n/translations/ja.yml

web/src/i18n/translations/en.yml Show resolved Hide resolved
@caichi-t caichi-t merged commit b34b98e into main Nov 14, 2024
22 checks passed
@caichi-t caichi-t deleted the fix-web/float-field branch November 14, 2024 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants