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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

feat: add hex color code box #362

Merged
merged 2 commits into from
Jun 11, 2024
Merged

feat: add hex color code box #362

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20368dc
feat: add hex color code box
harini-1597 Jun 7, 2024
b463b9e
fix: updated colorpicker
harini-1597 Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 53 additions & 57 deletions src/components/core/colorPicker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,38 @@ import { useState } from "react";

type ColorpickerTypes = {
colors: {
color01: string;
color02: string;
color03: string;
color04: string;
[key: string]: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure the dynamic key type is properly validated.

The dynamic key in the colors type declaration allows any string as a key, which could lead to potential runtime errors if keys do not match expected patterns. Consider adding more specific validation or constraints on the keys used in this object.

};
isPalleteTouched: boolean;
setColors: any;
setTouched: any;
};


// eslint-disable-next-line react/prop-types
function Colorpicker({ colors, setColors, setTouched, isPalleteTouched }: ColorpickerTypes) {
const [selectedColorSection, setColorSection] = useState<number | null>(null);
const [colorInputs, setColorInputs] = useState(colors);

const changeHandler = (color: string) => {
if (!isPalleteTouched) {
setTouched(true);
}
if (typeof color === "string")
setColors({ ...colors, [`color0` + selectedColorSection]: color });
if (typeof color === "string") {
const updatedColors = { ...colors, [`color0${selectedColorSection}`]: color };
setColors(updatedColors);
setColorInputs(updatedColors);
}
Comment on lines +17 to +27
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor to simplify state management in changeHandler.

Consider combining setColors and setColorInputs into a single state update function to reduce redundancy and potential bugs.

- setColors({ ...colors, [`color0${selectedColorSection}`]: color });
- setColorInputs({ ...colorInputs, [`color0${selectedColorSection}`]: color });
+ const updatedColors = { ...colors, [`color0${selectedColorSection}`]: color };
+ setColors(updatedColors);
+ setColorInputs(updatedColors);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const [colorInputs, setColorInputs] = useState(colors);
const changeHandler = (color: string) => {
if (!isPalleteTouched) {
setTouched(true);
}
if (typeof color === "string")
setColors({ ...colors, [`color0` + selectedColorSection]: color });
if (typeof color === "string") {
const updatedColors = { ...colors, [`color0${selectedColorSection}`]: color };
setColors(updatedColors);
setColorInputs(updatedColors);
}
const [colorInputs, setColorInputs] = useState(colors);
const changeHandler = (color: string) => {
if (!isPalleteTouched) {
setTouched(true);
}
if (typeof color === "string") {
const updatedColors = { ...colors, [`color0${selectedColorSection}`]: color };
setColors(updatedColors);
setColorInputs(updatedColors);
}

};

const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>, section: number) => {
const value = e.target.value;
setColorInputs({ ...colorInputs, [`color0${section}`]: value });
if (/^#([0-9A-F]{3}){1,2}$/i.test(value)) {
changeHandler(value);
}
Comment on lines +30 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

Validate HEX color codes more robustly in handleInputChange.

The current regex does not support 3-character HEX codes. Consider supporting both 3 and 6-character codes.

- if (/^#([0-9A-F]{3}){1,2}$/i.test(value)) {
+ if (/^#([0-9A-F]{3}){1,2}$/i.test(value)) {

Committable suggestion was skipped due to low confidence.

};

const showPicker = (id: number) => {
if (id === selectedColorSection) {
setColorSection(null);
Expand All @@ -33,59 +45,46 @@ function Colorpicker({ colors, setColors, setTouched, isPalleteTouched }: Colorp

return (
<>
<div className="relative">
<div className="w-[300px] rounded-[0.8rem] h-[200px] bg-[#888A8A] mx-auto flex">
<div
onClick={() => showPicker(1)}
className="cursor-pointer w-full rounded-tl-[0.8rem] rounded-bl-[0.8rem]"
style={{
backgroundColor: (typeof colors.color01 === "string" && colors.color01) || "",
}}
></div>
<div
onClick={() => showPicker(2)}
className="cursor-pointer w-full "
style={{
backgroundColor: (typeof colors.color02 === "string" && colors.color02) || "",
}}
></div>
<div
onClick={() => showPicker(3)}
className="cursor-pointer w-full"
style={{
backgroundColor: (typeof colors.color03 === "string" && colors.color03) || "",
}}
></div>
<div
onClick={() => showPicker(4)}
className="cursor-pointer w-full rounded-tr-[0.8rem] rounded-br-[0.8rem]"
style={{
backgroundColor: (typeof colors.color04 === "string" && colors.color04) || "",
}}
></div>
<div className="relative mx-auto">
<div className="relative pl-[70px] pr-2">
<div className="mx-auto w-[400px] rounded-[0.8rem] h-[175px] bg-[#888A8A] flex">
{['color01', 'color02', 'color03', 'color04'].map((color, index) => (
<div
key={color}
onClick={() => showPicker(index + 1)}
onKeyUp={(e) => { if (e.key === 'Enter') showPicker(index + 1); }}
className={`cursor-pointer w-full ${index === 0 ? 'rounded-tl-[0.8rem] rounded-bl-[0.8rem]' : ''} ${index === 3 ? 'rounded-tr-[0.8rem] rounded-br-[0.8rem]' : ''}`}
style={{
backgroundColor: colors[color],
}}
></div>
Comment on lines +52 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Use self-closing tags for empty JSX elements.

JSX elements without children should be marked as self-closing to improve readability and maintain consistency with JSX style guidelines.

- <div style={{ backgroundColor: colors[color] }}></div>
+ <div style={{ backgroundColor: colors[color] }} />
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div
key={color}
onClick={() => showPicker(index + 1)}
onKeyUp={(e) => { if (e.key === 'Enter') showPicker(index + 1); }}
className={`cursor-pointer w-full ${index === 0 ? 'rounded-tl-[0.8rem] rounded-bl-[0.8rem]' : ''} ${index === 3 ? 'rounded-tr-[0.8rem] rounded-br-[0.8rem]' : ''}`}
style={{
backgroundColor: colors[color],
}}
></div>
<div
key={color}
onClick={() => showPicker(index + 1)}
onKeyUp={(e) => { if (e.key === 'Enter') showPicker(index + 1); }}
className={`cursor-pointer w-full ${index === 0 ? 'rounded-tl-[0.8rem] rounded-bl-[0.8rem]' : ''} ${index === 3 ? 'rounded-tr-[0.8rem] rounded-br-[0.8rem]' : ''}`}
style={{
backgroundColor: colors[color],
}}
/>
Tools
Biome

[error] 52-60: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. (lint/style/useSelfClosingElements)

Unsafe fix: Use a SelfClosingElement instead

))}
</div>
<div className="flex justify-center mt-1 mx-20">
{['color01', 'color02', 'color03', 'color04'].map((color, index) => (
<input
key={color}
type="text"
value={colorInputs[color]}
onChange={(e) => handleInputChange(e, index + 1)}
placeholder="#000000"
className="w-20 text-center mx-2.5"
/>
))}
</div>
</div>
{selectedColorSection && (
<>
{/* <div className='fixed top-0 left-0 right-0 bottom-0' onClick={handleClose}></div> */}
<div className="absolute top-0">
<HexColorPicker
style={{ width: "100px" }}
onChange={changeHandler}
color={
(selectedColorSection &&
((selectedColorSection === 1 && colors["color01"]) ||
(selectedColorSection === 2 && colors["color02"]) ||
(selectedColorSection === 3 && colors["color03"]) ||
(selectedColorSection === 4 && colors["color04"]))) ||
""
}
/>
</div>
</>
<div className="absolute top-0">
<HexColorPicker
style={{ width: "100px" }}
onChange={changeHandler}
color={colors[`color0${selectedColorSection}`]}
/>
</div>
)}
</div>

{/* <div className="grid grid-cols-1 md:grid-cols-2 md:grid-rows-2 gap-4">
{/* <div className="grid grid-cols-1 md:grid-cols-2 md:grid-rows-2 gap-4">

<div className="mb-3">
<input
Expand All @@ -100,7 +99,6 @@ function Colorpicker({ colors, setColors, setTouched, isPalleteTouched }: Colorp
maxLength={6}
/>
</div>

<div className="mb-3">
<input
type="text"
Expand All @@ -114,7 +112,6 @@ function Colorpicker({ colors, setColors, setTouched, isPalleteTouched }: Colorp
maxLength={6}
/>
</div>

<div className="mb-3">
<input
type="text"
Expand All @@ -128,7 +125,6 @@ function Colorpicker({ colors, setColors, setTouched, isPalleteTouched }: Colorp
maxLength={6}
/>
</div>

<div className="mb-3">
<input
type="text"
Expand Down
Loading