chore re-organized types #2338
chore re-organized types #2338
Conversation
| import Text from "../Text/Text"; | ||
| import Flex from "../Flex/Flex"; | ||
| import styles from "./AttentionBox.module.scss"; | ||
| import { SubIcon } from "../Icon"; |
There was a problem hiding this comment.
is it out of "types" in purpose?
| import styles from "./Avatar.module.scss"; | ||
| import { SubIcon } from "../Icon"; |
There was a problem hiding this comment.
same, and for the rest usages
There was a problem hiding this comment.
it is a bit weird it is inside Box, isn't it? because in theory it is a very general type (we should maybe try to make this kind of general types file, to save us redundant re-writing)
There was a problem hiding this comment.
this file also exports a function, do you think it can benefit to have a utils file as well and send it to there? so we would better separate types from utils
| @@ -3,11 +3,11 @@ import cx from "classnames"; | |||
| import React, { CSSProperties, forwardRef, Ref } from "react"; | |||
| import useMergeRef from "../../hooks/useMergeRef"; | |||
| import { IconType as IconTypeEnum } from "./IconConstants"; | |||
| import { IconType } from "./Icon.types"; | |||
| import { IconType, MouseEventCallBack, SubIcon } from "./Icon.types"; | |||
There was a problem hiding this comment.
also sound like something very generic that should be in types rather in Icon.types (MouseEventCallBack)
| export type IconType = "svg" | "font" | "src"; | ||
|
|
||
| // Custom type for all mouse event callbacks | ||
| export type MouseEventCallBack = (event: React.MouseEvent<HTMLElement | SVGElement>) => void; |
There was a problem hiding this comment.
I think it shouldn't be a different type. I think this should be inlined:
onClick?: (event: React.MouseEvent<HTMLElement | SVGElement>) => void;
There was a problem hiding this comment.
since it is being used only once I don't see much value in making a type for it
There was a problem hiding this comment.
maybe even better, and make this type even more redundant:
onClick?: MouseEventHandler<HTMLElement | SVGElement>
|
|
||
| type IconPath = `/icons/${keyof typeof allIcons}`; | ||
| type SplittedIconPath = SplitString<IconPath, "/">; | ||
| type SplittedIconPath = Helpers<IconPath, "/">; |
There was a problem hiding this comment.
is this intended? should it be called Helpers? I don't understand what exactly the helpers type does, but maybe a more meaningful name like SplitString can better explain what it does
There was a problem hiding this comment.
I'm not actually familiar with what's going on there, but Helpers sound too generic
| @@ -1 +1 @@ | |||
| export type Sizes = "small" | "medium"; | |||
| export type ToggleSizes = "small" | "medium"; | |||
There was a problem hiding this comment.
| export type ToggleSizes = "small" | "medium"; | |
| export type ToggleSize = "small" | "medium"; |
wdyt?
https://monday.monday.com/boards/3532714909/views/80492480/pulses/7160133818