Minor UI/UX improvement #534
Conversation
…ield is shown in pen/line/arrow, pin color icon in modal is more aligned.
WalkthroughThis update introduces a new prop, Changes
Sequence Diagram(s)sequenceDiagram
participant ToolOption as Tool Option Component (e.g., EllipseOptions)
participant AnnotationOptions
participant StrokeFields
participant ColorInput
ToolOption->>AnnotationOptions: Render with hideStrokeField=true
AnnotationOptions-->>AnnotationOptions: Hide stroke option logic/UI
AnnotationOptions-->>StrokeFields: (Only if hideStrokeField=false) Render StrokeFields
StrokeFields->>StrokeFields: Render Slider and StyledSliderInput (sync strokeWidth)
AnnotationOptions-->>ColorInput: Render ColorInput for fill (if not hidden)
ColorInput-->>ColorInput: Load pinned colors (from localStorage or default)
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/react-filerobot-image-editor/src/components/common/ColorInput/index.jsx (1)
44-57: Code formatting issue in the conditionalThe code looks functionally sound with good localStorage handling, but there's a formatting issue with the if statement indentation.
- if (JSON.stringify(newPinnedColors) !== JSON.stringify(currentPinnedColors)) { + if ( + JSON.stringify(newPinnedColors) !== JSON.stringify(currentPinnedColors) + ) {🧰 Tools
🪛 ESLint
[error] 52-53: Replace
⏎····if·(JSON.stringify(newPinnedColors)·!==·JSON.stringify(currentPinnedColors)with····if·(⏎······JSON.stringify(newPinnedColors)·!==·JSON.stringify(currentPinnedColors)⏎····(prettier/prettier)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx (2)
10-10: Fix import ordering to follow project conventionsThe import statement for
StyledSliderInputshould be placed before the local imports./** Internal Dependencies */ import restrictNumber from 'utils/restrictNumber'; import ColorInput from 'components/common/ColorInput'; +import { StyledSliderInput } from 'components/tools/tools.styled'; import { StyledSpacedOptionFields } from './AnnotationOptions.styled'; import Slider from '../Slider'; -import { StyledSliderInput } from 'components/tools/tools.styled';🧰 Tools
🪛 ESLint
[error] 10-10:
components/tools/tools.styledimport should occur before import of./AnnotationOptions.styled(import/order)
34-37: Great enhancement: Added numeric input field for precise stroke width controlAdding a direct input field alongside the slider improves usability by allowing users to enter exact stroke width values. However, there are some formatting issues with indentation.
- <StyledSliderInput - value={strokeWidth} - onChange={({ target: { value } }) => changeStrokeWidth(value)} - /> + <StyledSliderInput + value={strokeWidth} + onChange={({ target: { value } }) => changeStrokeWidth(value)} + />🧰 Tools
🪛 ESLint
[error] 35-35: Delete
··(prettier/prettier)
[error] 36-36: Delete
··(prettier/prettier)
[error] 37-37: Delete
··(prettier/prettier)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx (1)
129-130: Fix formatting issues with conditional renderingThere are some formatting issues with the conditional rendering of StrokeFields and spacing before the hideFillOption conditional.
- { !hideStrokeField && <StrokeFields annotation={annotation} updateAnnotation={updateAnnotation} /> } - { !hideFillOption && ( + {!hideStrokeField && ( + <StrokeFields + annotation={annotation} + updateAnnotation={updateAnnotation} + /> + )} + {!hideFillOption && (🧰 Tools
🪛 ESLint
[error] 129-129: Replace
·!hideStrokeField·&&·<StrokeFields·annotation={annotation}·updateAnnotation={updateAnnotation}·/>·with!hideStrokeField·&&·(⏎········<StrokeFields⏎··········annotation={annotation}⏎··········updateAnnotation={updateAnnotation}⏎········/>⏎······)(prettier/prettier)
[error] 130-130: Delete
·(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx(2 hunks)packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx(6 hunks)packages/react-filerobot-image-editor/src/components/common/ColorInput/index.jsx(2 hunks)packages/react-filerobot-image-editor/src/components/common/ColorPickerModal/ColorPickerModal.styled.js(0 hunks)packages/react-filerobot-image-editor/src/components/tools/Ellipse/EllipseOptions.jsx(1 hunks)packages/react-filerobot-image-editor/src/components/tools/Image/ImageControls.jsx(1 hunks)packages/react-filerobot-image-editor/src/components/tools/Polygon/PolygonOptions.jsx(1 hunks)packages/react-filerobot-image-editor/src/components/tools/Rect/RectOptions.jsx(1 hunks)packages/react-filerobot-image-editor/src/components/tools/Text/TextOptions/TextControls.jsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/react-filerobot-image-editor/src/components/common/ColorPickerModal/ColorPickerModal.styled.js
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx (1)
packages/react-filerobot-image-editor/src/components/tools/tools.styled.js (1)
StyledSliderInput(41-55)
🪛 ESLint
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx
[error] 10-10: components/tools/tools.styled import should occur before import of ./AnnotationOptions.styled
(import/order)
[error] 35-35: Delete ··
(prettier/prettier)
[error] 36-36: Delete ··
(prettier/prettier)
[error] 37-37: Delete ··
(prettier/prettier)
packages/react-filerobot-image-editor/src/components/common/ColorInput/index.jsx
[error] 52-53: Replace ⏎····if·(JSON.stringify(newPinnedColors)·!==·JSON.stringify(currentPinnedColors) with ····if·(⏎······JSON.stringify(newPinnedColors)·!==·JSON.stringify(currentPinnedColors)⏎····
(prettier/prettier)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx
[error] 56-65: Do not nest ternary expressions.
(no-nested-ternary)
[error] 62-64: Replace ⏎············{·titleKey:·'shadow',·name:·POPPABLE_OPTIONS.SHADOW,·Icon:·Shadow·},⏎·········· with {·titleKey:·'shadow',·name:·POPPABLE_OPTIONS.SHADOW,·Icon:·Shadow·}
(prettier/prettier)
[error] 129-129: Replace ·!hideStrokeField·&&·<StrokeFields·annotation={annotation}·updateAnnotation={updateAnnotation}·/>· with !hideStrokeField·&&·(⏎········<StrokeFields⏎··········annotation={annotation}⏎··········updateAnnotation={updateAnnotation}⏎········/>⏎······)
(prettier/prettier)
[error] 130-130: Delete ·
(prettier/prettier)
🔇 Additional comments (11)
packages/react-filerobot-image-editor/src/components/common/ColorInput/index.jsx (2)
13-20: Excellent addition of preset color options!Adding default preset colors improves the user experience by providing immediate color choices without requiring users to have previously pinned colors. The selection includes basic colors that cover primary use cases and matches the PR objective of enhancing UI/UX.
35-41: Good implementation of localStorage fallback mechanismThe state initialization now properly uses a lazy initializer with the new preset colors as fallback. This ensures users always have color options available even on first use.
packages/react-filerobot-image-editor/src/components/tools/Ellipse/EllipseOptions.jsx (1)
20-20: Appropriate hiding of stroke field for ellipse toolAdding the
hideStrokeFieldprop aligns with the PR's objective to improve UI consistency across different tools. This ensures that stroke-related UI elements are properly controlled based on the tool type.packages/react-filerobot-image-editor/src/components/tools/Polygon/PolygonOptions.jsx (1)
28-28: Consistent UI handling with hideStrokeFieldAdding the
hideStrokeFieldprop maintains consistency with other shape tools and improves the overall UI experience by controlling visibility of stroke-related options.packages/react-filerobot-image-editor/src/components/tools/Image/ImageControls.jsx (1)
15-15: Appropriate UI control for image toolAdding
hideStrokeFieldprop to match the pattern used in other tool components ensures consistent UI behavior across the application. This change properly aligns with the PR's UI/UX improvement goals.packages/react-filerobot-image-editor/src/components/tools/Text/TextOptions/TextControls.jsx (1)
128-128: LGTM: AddedhideStrokeFieldprop to hide stroke controls for text toolsThis addition makes sense as text tools typically work with fill color rather than stroke properties, aligning with the PR objective to make stroke fields consistently display only for relevant tools.
packages/react-filerobot-image-editor/src/components/tools/Rect/RectOptions.jsx (1)
26-26: LGTM: AddedhideStrokeFieldprop to hide stroke controls for rectangle toolsAdding the prop is consistent with the implementation in other shape tools, improving UI consistency across the application.
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx (4)
34-34: LGTM: AddedhideStrokeFieldprop for controlling stroke field visibilityThis prop allows for more granular control over which annotation tools display stroke-related UI elements, improving the user experience.
66-74: LGTM: Simplified conditional for position fieldsGood refactoring of the position fields logic, making it more concise and readable.
97-106: LGTM: Updated annotation fill handler based on hideStrokeField flagThis change intelligently updates either stroke or fill property based on the hideStrokeField flag, which aligns with the PR objective of improving stroke field visibility for relevant tools.
188-188: LGTM: Added default prop and prop type for hideStrokeFieldProperly defined the default value (false) and prop type (bool) for the new hideStrokeField prop.
Also applies to: 198-198
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx (1)
34-37: Good UX improvement: Added numeric input for precise stroke width control.Adding a numeric input alongside the slider provides users with a more precise way to control stroke width. This improves usability by allowing exact numeric values to be entered directly.
However, there are spacing/indentation issues flagged by ESLint that should be fixed:
- <StyledSliderInput - value={strokeWidth} - onChange={({ target: { value } }) => changeStrokeWidth(value)} - /> + <StyledSliderInput + value={strokeWidth} + onChange={({ target: { value } }) => changeStrokeWidth(value)} + />🧰 Tools
🪛 ESLint
[error] 35-35: Delete
··(prettier/prettier)
[error] 36-36: Delete
··(prettier/prettier)
[error] 37-37: Delete
··(prettier/prettier)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx (3)
95-99: Behavior change inchangeAnnotationFillfunction.The function now updates either the
strokeorfillproperty based on thehideStrokeFieldflag. While this works, the function namechangeAnnotationFilldoesn't accurately reflect its dual purpose, which could be confusing for future maintenance.Consider renaming the function to better reflect its purpose or refactoring to separate concerns:
- const changeAnnotationFill = useCallback( + const changeAnnotationColor = useCallback( (newFill) => { if (!hideStrokeField) { updateAnnotation({ stroke: newFill }); } else { updateAnnotation({ fill: newFill }); } }, - [updateAnnotation], + [updateAnnotation, hideStrokeField], );
125-130: Good conditional rendering of StrokeFields component.The conditional rendering of the
StrokeFieldscomponent based on thehideStrokeFieldprop is correctly implemented, though there are some formatting issues:- { !hideStrokeField && ( - <StrokeFields - annotation={annotation} - updateAnnotation={updateAnnotation} - /> - )} + {!hideStrokeField && ( + <StrokeFields + annotation={annotation} + updateAnnotation={updateAnnotation} + /> + )}🧰 Tools
🪛 ESLint
[error] 125-125: Delete
·(prettier/prettier)
[error] 126-126: Delete
·(prettier/prettier)
[error] 127-127: Delete
·(prettier/prettier)
[error] 128-128: Delete
·(prettier/prettier)
[error] 129-129: Delete
·(prettier/prettier)
131-131: Formatting issue in conditional rendering.There's an extra space before the opening brace:
- { !hideFillOption && ( + {!hideFillOption && (🧰 Tools
🪛 ESLint
[error] 131-131: Delete
·(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx(2 hunks)packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx (1)
packages/react-filerobot-image-editor/src/components/tools/tools.styled.js (1)
StyledSliderInput(41-55)
🪛 ESLint
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx
[error] 35-35: Delete ··
(prettier/prettier)
[error] 36-36: Delete ··
(prettier/prettier)
[error] 37-37: Delete ··
(prettier/prettier)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx
[error] 58-58: Replace ·?·[{·titleKey:·'stroke',·name:·POPPABLE_OPTIONS.STROKE,·Icon:·Stroke·}] with ⏎··············?·[⏎··················{⏎····················titleKey:·'stroke',⏎····················name:·POPPABLE_OPTIONS.STROKE,⏎····················Icon:·Stroke,⏎··················},⏎················]⏎·············
(prettier/prettier)
[error] 125-125: Delete ·
(prettier/prettier)
[error] 126-126: Delete ·
(prettier/prettier)
[error] 127-127: Delete ·
(prettier/prettier)
[error] 128-128: Delete ·
(prettier/prettier)
[error] 129-129: Delete ·
(prettier/prettier)
[error] 131-131: Delete ·
(prettier/prettier)
🔇 Additional comments (4)
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/StrokeFields.jsx (1)
8-8: Good import addition for the new input component.The import for
StyledSliderInputis correctly added to support the new numeric input field for stroke width.packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx (3)
34-34: Good addition of new configuration prop.Adding the
hideStrokeFieldprop provides a clean way to control the visibility of stroke-related UI elements across different annotation tools.
62-70: Good simplification of conditional rendering for position fields.The refactored code improves readability by using a more straightforward conditional spread operator pattern for position-related options.
189-189: Properly added default prop and prop type.The
hideStrokeFieldprop is correctly added to both defaultProps and propTypes, with a sensible default value offalse.Also applies to: 199-199
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx
Show resolved
Hide resolved
packages/react-filerobot-image-editor/src/components/common/AnnotationOptions/index.jsx
Show resolved
Hide resolved
|
|
||
| return ( | ||
| <StyledSpacedOptionFields> | ||
| <StyledSliderInput |
There was a problem hiding this comment.
what's the difference between this and the next Slider call?
There was a problem hiding this comment.
This is an Input that shows the width makes it easier to write strokeWidth if the slider is to hard to slide and find a specific width. Also you dont know exactly what width you have unless you hover over so that helps too(specially in mobile mode).
2 participants
Minor UI/UX improvement, preset colors added in color-picker, strokeField is shown in pen/line/arrow, pin color icon in modal is more aligned.
Preset colors added and pin color aligned:
Stroke field is always shown for pen/line/arrow for better user experience:
Summary by CodeRabbit