-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[WEB-2799] chore: global component and code refactor #6131
Changes from all commits
15c57f8
ad80fb5
1b0854b
0eddccc
6a1a32f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
export * from "./use-local-storage"; | ||
export * from "./use-outside-click-detector"; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,59 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { useState, useEffect, useCallback } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const getValueFromLocalStorage = (key: string, defaultValue: any) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (typeof window === undefined || typeof window === "undefined") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return defaultValue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const item = window.localStorage.getItem(key); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return item ? JSON.parse(item) : defaultValue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.localStorage.removeItem(key); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return defaultValue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const setValueIntoLocalStorage = (key: string, value: any) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (typeof window === undefined || typeof window === "undefined") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.localStorage.setItem(key, JSON.stringify(value)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+15
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add type safety and improve error handling Similar improvements needed as the get function:
Apply these changes: -export const setValueIntoLocalStorage = (key: string, value: any) => {
- if (typeof window === undefined || typeof window === "undefined")
+export const setValueIntoLocalStorage = <T>(key: string, value: T): boolean => {
+ if (typeof window === "undefined")
return false;
try {
window.localStorage.setItem(key, JSON.stringify(value));
return true;
} catch (error) {
+ console.warn(`Error writing ${key} to localStorage:`, error);
return false;
}
}; 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 16-16: Invalid not a string literal (lint/suspicious/useValidTypeof) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const useLocalStorage = <T,>(key: string, initialValue: T) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [storedValue, setStoredValue] = useState<T | null>(() => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
getValueFromLocalStorage(key, initialValue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const setValue = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(value: T) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.localStorage.setItem(key, JSON.stringify(value)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setStoredValue(value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.dispatchEvent(new Event(`local-storage:${key}`)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[key] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const clearValue = useCallback(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.localStorage.removeItem(key); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setStoredValue(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.dispatchEvent(new Event(`local-storage:${key}`)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, [key]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const reHydrate = useCallback(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const data = getValueFromLocalStorage(key, initialValue); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setStoredValue(data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, [key, initialValue]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.addEventListener(`local-storage:${key}`, reHydrate); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.removeEventListener(`local-storage:${key}`, reHydrate); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, [key, reHydrate]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return { storedValue, setValue, clearValue } as const; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+26
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error handling and type safety in the hook Several improvements needed in the hook implementation:
Apply these changes: export const useLocalStorage = <T,>(key: string, initialValue: T) => {
const [storedValue, setStoredValue] = useState<T | null>(() =>
getValueFromLocalStorage(key, initialValue)
);
const setValue = useCallback(
(value: T) => {
+ try {
window.localStorage.setItem(key, JSON.stringify(value));
setStoredValue(value);
window.dispatchEvent(new Event(`local-storage:${key}`));
+ } catch (error) {
+ console.warn(`Error in setValue for ${key}:`, error);
+ }
},
[key]
);
const clearValue = useCallback(() => {
+ try {
window.localStorage.removeItem(key);
setStoredValue(null);
window.dispatchEvent(new Event(`local-storage:${key}`));
+ } catch (error) {
+ console.warn(`Error in clearValue for ${key}:`, error);
+ }
}, [key]);
const reHydrate = useCallback(() => {
const data = getValueFromLocalStorage(key, initialValue);
setStoredValue(data);
}, [key, initialValue]);
useEffect(() => {
+ const eventName = `local-storage:${key}`;
+ const handler = () => reHydrate();
+
- window.addEventListener(`local-storage:${key}`, reHydrate);
+ window.addEventListener(eventName, handler);
return () => {
- window.removeEventListener(`local-storage:${key}`, reHydrate);
+ window.removeEventListener(eventName, handler);
};
}, [key, reHydrate]);
return { storedValue, setValue, clearValue } as const;
}; 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import * as React from "react"; | ||
|
||
import { ISvgIcons } from "./type"; | ||
|
||
export const CommentFillIcon: React.FC<ISvgIcons> = ({ className = "text-current", ...rest }) => ( | ||
<svg viewBox="0 0 24 24" className={`${className}`} xmlns="http://www.w3.org/2000/svg" {...rest}> | ||
<path | ||
fillRule="evenodd" | ||
clipRule="evenodd" | ||
d="M4.848 2.771C7.21613 2.4234 9.60649 2.24927 12 2.25C14.43 2.25 16.817 2.428 19.152 2.77C21.13 3.062 22.5 4.794 22.5 6.74V12.76C22.5 14.706 21.13 16.438 19.152 16.73C17.212 17.014 15.236 17.185 13.23 17.235C13.1303 17.2369 13.0351 17.277 12.964 17.347L8.78 21.53C8.67511 21.6348 8.54153 21.7061 8.39614 21.735C8.25074 21.7638 8.10004 21.749 7.96308 21.6923C7.82611 21.6356 7.70903 21.5395 7.62661 21.4163C7.54419 21.2931 7.50013 21.1482 7.5 21V17.045C6.61329 16.9639 5.72895 16.8585 4.848 16.729C2.87 16.439 1.5 14.705 1.5 12.759V6.741C1.5 4.795 2.87 3.061 4.848 2.771Z" | ||
fill="currentColor" | ||
/> | ||
</svg> | ||
); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,28 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import * as React from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export type Props = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
className?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
width?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
height?: string | number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
color?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+3
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Standardize props interface with other icons The component uses a custom Props type while other icons use ISvgIcons. Also, the color prop is defined but unused. -export type Props = {
- className?: string;
- width?: string | number;
- height?: string | number;
- color?: string;
-};
+import { ISvgIcons } from "./type";
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const EpicIcon: React.FC<Props> = ({ width = "16", height = "16", className }) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<svg | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
width={width} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
height={height} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
className={className} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
viewBox="0 0 16 16" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fill="none" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
xmlns="http://www.w3.org/2000/svg" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
d="M0.900146 9.33203V12.0142C0.900146 12.3736 1.17392 12.6654 1.51126 12.6654H14.9557C15.1178 12.6654 15.2732 12.5968 15.3878 12.4746C15.5024 12.3525 15.5668 12.1869 15.5668 12.0142V10.3299L13.375 7.99523C13.1458 7.75134 12.8351 7.61436 12.5113 7.61436C12.1874 7.61436 11.8767 7.75134 11.6476 7.99523L10.1257 9.35919L10.2534 9.56204L11.7209 9.60056C11.7809 9.66017 11.8291 9.73206 11.8625 9.81194C11.8959 9.89181 11.9138 9.97804 11.9153 10.0655C11.9167 10.1529 11.9017 10.2397 11.8709 10.3208C11.8402 10.4019 11.7944 10.4756 11.7364 10.5374C11.6784 10.5992 11.6092 10.648 11.5332 10.6807C11.4571 10.7135 11.3756 10.7296 11.2935 10.728C11.2114 10.7265 11.1305 10.7073 11.0556 10.6717C10.9806 10.6362 10.9131 10.5848 10.8572 10.5209L10.2534 9.56204L6.60385 3.76614C6.37468 3.52226 6.11293 3.33203 5.78904 3.33203C5.46515 3.33203 5.20339 3.52226 4.97422 3.76614L0.900146 9.33203Z" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fill="currentColor" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
d="M11.7209 9.60056L10.2534 9.56204L10.8572 10.5209C10.9131 10.5848 10.9806 10.6362 11.0556 10.6717C11.1305 10.7073 11.2114 10.7265 11.2935 10.728C11.3756 10.7296 11.4571 10.7135 11.5332 10.6807C11.6092 10.648 11.6784 10.5992 11.7364 10.5374C11.7944 10.4756 11.8402 10.4019 11.8709 10.3208C11.9017 10.2397 11.9167 10.1529 11.9153 10.0655C11.9138 9.97804 11.8959 9.89181 11.8625 9.81194C11.8291 9.73206 11.7809 9.66017 11.7209 9.60056Z" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fill="currentColor" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</svg> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+10
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Optimize SVG structure and add accessibility
-export const EpicIcon: React.FC<Props> = ({ width = "16", height = "16", className }) => (
+export const EpicIcon: React.FC<ISvgIcons> = ({ className = "text-current", ...rest }) => (
<svg
- width={width}
- height={height}
className={className}
viewBox="0 0 16 16"
- fill="none"
xmlns="http://www.w3.org/2000/svg"
+ aria-hidden="true"
+ role="img"
+ {...rest}
> 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import * as React from "react"; | ||
|
||
import { ISvgIcons } from "./type"; | ||
|
||
export const InfoFillIcon: React.FC<ISvgIcons> = ({ className = "text-current", ...rest }) => ( | ||
<svg viewBox="0 0 24 24" className={`${className}`} xmlns="http://www.w3.org/2000/svg" {...rest}> | ||
<path | ||
fillRule="evenodd" | ||
clipRule="evenodd" | ||
d="M2.25 12C2.25 6.615 6.615 2.25 12 2.25C17.385 2.25 21.75 6.615 21.75 12C21.75 17.385 17.385 21.75 12 21.75C6.615 21.75 2.25 17.385 2.25 12ZM10.956 10.558C12.102 9.985 13.393 11.021 13.082 12.264L12.373 15.1L12.415 15.08C12.5912 15.0025 12.7905 14.9958 12.9715 15.0612C13.1526 15.1265 13.3016 15.259 13.3877 15.4312C13.4737 15.6033 13.4903 15.802 13.434 15.9861C13.3777 16.1702 13.2527 16.3255 13.085 16.42L13.045 16.442C11.898 17.015 10.607 15.979 10.918 14.736L11.628 11.9L11.586 11.92C11.4975 11.9692 11.4 11.9999 11.2994 12.0104C11.1987 12.0209 11.097 12.0109 11.0003 11.981C10.9036 11.9511 10.8139 11.902 10.7367 11.8366C10.6595 11.7711 10.5964 11.6907 10.551 11.6002C10.5057 11.5098 10.4792 11.411 10.4731 11.31C10.4669 11.209 10.4813 11.1078 10.5153 11.0124C10.5493 10.9171 10.6022 10.8297 10.6709 10.7553C10.7396 10.681 10.8226 10.6214 10.915 10.58L10.956 10.558ZM12 9C12.1989 9 12.3897 8.92098 12.5303 8.78033C12.671 8.63968 12.75 8.44891 12.75 8.25C12.75 8.05109 12.671 7.86032 12.5303 7.71967C12.3897 7.57902 12.1989 7.5 12 7.5C11.8011 7.5 11.6103 7.57902 11.4697 7.71967C11.329 7.86032 11.25 8.05109 11.25 8.25C11.25 8.44891 11.329 8.63968 11.4697 8.78033C11.6103 8.92098 11.8011 9 12 9Z" | ||
fill="currentColor" | ||
/> | ||
</svg> | ||
); | ||
Comment on lines
+5
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider icon component abstraction and add accessibility
export const InfoFillIcon: React.FC<ISvgIcons> = ({ className = "text-current", ...rest }) => (
- <svg viewBox="0 0 24 24" className={`${className}`} xmlns="http://www.w3.org/2000/svg" {...rest}>
+ <svg
+ viewBox="0 0 24 24"
+ className={`${className}`}
+ xmlns="http://www.w3.org/2000/svg"
+ aria-hidden="true"
+ role="img"
+ {...rest}
+ > Consider creating a base icon component: type BaseIconProps = ISvgIcons & {
children: React.ReactNode;
viewBox?: string;
};
const BaseIcon: React.FC<BaseIconProps> = ({
className = "text-current",
viewBox = "0 0 24 24",
children,
...rest
}) => (
<svg
viewBox={viewBox}
className={className}
xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
role="img"
{...rest}
>
{children}
</svg>
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from "./tabs"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
import React, { FC, Fragment } from "react"; | ||
import { Tab } from "@headlessui/react"; | ||
import { LucideProps } from "lucide-react"; | ||
// helpers | ||
import { useLocalStorage } from "@plane/helpers"; | ||
import { cn } from "../../helpers"; | ||
|
||
type TabItem = { | ||
key: string; | ||
icon?: FC<LucideProps>; | ||
label?: React.ReactNode; | ||
content: React.ReactNode; | ||
disabled?: boolean; | ||
}; | ||
|
||
type TTabsProps = { | ||
tabs: TabItem[]; | ||
storageKey: string; | ||
actions?: React.ReactNode; | ||
defaultTab?: string; | ||
containerClassName?: string; | ||
tabListContainerClassName?: string; | ||
tabListClassName?: string; | ||
tabClassName?: string; | ||
tabPanelClassName?: string; | ||
}; | ||
|
||
export const Tabs: FC<TTabsProps> = (props: TTabsProps) => { | ||
const { | ||
tabs, | ||
storageKey, | ||
actions, | ||
defaultTab = tabs[0]?.key, | ||
containerClassName = "", | ||
tabListContainerClassName = "", | ||
tabListClassName = "", | ||
tabClassName = "", | ||
tabPanelClassName = "", | ||
} = props; | ||
// local storage | ||
const { storedValue, setValue } = useLocalStorage(`tab-${storageKey}`, defaultTab); | ||
|
||
const currentTabIndex = (tabKey: string): number => tabs.findIndex((tab) => tab.key === tabKey); | ||
|
||
Comment on lines
+28
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling and validation The component needs better error handling:
Consider these improvements: +import { ErrorBoundary } from "react-error-boundary";
export const Tabs: FC<TTabsProps> = (props: TTabsProps) => {
const {
tabs,
storageKey,
actions,
- defaultTab = tabs[0]?.key,
+ defaultTab,
containerClassName = "",
tabListContainerClassName = "",
tabListClassName = "",
tabClassName = "",
tabPanelClassName = "",
} = props;
+ if (!tabs.length) {
+ return null;
+ }
+ const effectiveDefaultTab = defaultTab || tabs[0].key;
+
// local storage
- const { storedValue, setValue } = useLocalStorage(`tab-${storageKey}`, defaultTab);
+ const { storedValue, setValue } = useLocalStorage(`tab-${storageKey}`, effectiveDefaultTab);
const currentTabIndex = (tabKey: string): number => tabs.findIndex((tab) => tab.key === tabKey);
|
||
return ( | ||
<div className="flex flex-col w-full h-full"> | ||
<Tab.Group defaultIndex={currentTabIndex(storedValue ?? defaultTab)}> | ||
<div className={cn("flex flex-col w-full h-full gap-2", containerClassName)}> | ||
<div className={cn("flex w-full items-center gap-4", tabListContainerClassName)}> | ||
<Tab.List | ||
as="div" | ||
className={cn( | ||
"flex w-full min-w-fit items-center justify-between gap-1.5 rounded-md text-sm p-0.5 bg-custom-background-80/60", | ||
tabListClassName | ||
)} | ||
> | ||
{tabs.map((tab) => ( | ||
<Tab | ||
className={({ selected }) => | ||
cn( | ||
`flex items-center justify-center p-1 min-w-fit w-full font-medium text-custom-text-100 outline-none focus:outline-none cursor-pointer transition-all rounded`, | ||
selected | ||
? "bg-custom-background-100 text-custom-text-100 shadow-sm" | ||
: tab.disabled | ||
? "text-custom-text-400 cursor-not-allowed" | ||
: "text-custom-text-400 hover:text-custom-text-300 hover:bg-custom-background-80/60", | ||
tabClassName | ||
) | ||
} | ||
key={tab.key} | ||
onClick={() => { | ||
if (!tab.disabled) setValue(tab.key); | ||
}} | ||
disabled={tab.disabled} | ||
> | ||
{tab.icon && <tab.icon className="size-4" />} | ||
{tab.label} | ||
</Tab> | ||
))} | ||
</Tab.List> | ||
{actions && <div className="flex-grow">{actions}</div>} | ||
</div> | ||
<Tab.Panels as={Fragment}> | ||
{tabs.map((tab) => ( | ||
<Tab.Panel key={tab.key} as="div" className={cn("relative outline-none", tabPanelClassName)}> | ||
{tab.content} | ||
</Tab.Panel> | ||
))} | ||
</Tab.Panels> | ||
</div> | ||
</Tab.Group> | ||
</div> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve type safety and error handling
Several improvements can be made to this function:
Apply these changes:
📝 Committable suggestion
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: Invalid
typeof
comparison value: this expression is not a string literalnot a string literal
Unsafe fix: Compare the result of
typeof
with a valid type name(lint/suspicious/useValidTypeof)