Skip to content

Commit

Permalink
change: [M3-7232] Rename isPropValid and update codebase & documentat…
Browse files Browse the repository at this point in the history
…ion (#9790)

* Rename isValidProp and update codebase

* Added changeset: Rename isPropValid utility

* Feedback

* Update doc

* More feedback

* Post rebase fix

* Copy feedback
  • Loading branch information
abailly-akamai authored Oct 12, 2023
1 parent 5e023c7 commit d542062
Show file tree
Hide file tree
Showing 42 changed files with 221 additions and 160 deletions.
43 changes: 29 additions & 14 deletions docs/development-guide/02-component-structure.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,59 +7,74 @@ The basic structure of a component file should follow:
```
[ imports ]
[ types and interfaces ]
[ function component definition ]
[ styles ]
[ utility functions ]
[ default export ]
[ exported function component definition ]
[ styles ] (possibly in their own file)
[ utility functions ] (possibly in their own file)
```

Here is a minimal code example demonstrating the basic structure of a component file:

```tsx
import * as React from "react";
import { styled } from "@mui/material/styles";
import { isPropValid } from "src/utilities/isPropValid";
import { omittedProps } from "src/utilities/omittedProps";

interface SayHelloProps {
// If not exported, it can just be named `Props`
export interface SayHelloProps {
name: string;
isDisabled: boolean;
}

const SayHello = (props: SayHelloProps) => {
export const SayHello = (props: SayHelloProps) => {
const { name, isDisabled } = props;

return <StyledH1 isDisabled={isDisabled}>Hello, {capitalize(name)}</StyledH1>;
};

/**
* Should be moved to SayHello.styles.ts if component was large (> 100 lines).
*/
const StyledH1 = styled("h1", {
label: "StyledH1",
shouldForwardProp: (prop) => isPropValid(["isDisabled"], prop),
shouldForwardProp: omittedProps(["isDisabled"]),
})(({ theme, ...props }) => ({
color: props.isDisabled ? theme.color.grey : theme.color.black,
}));

/**
* It's often a good idea to move utilities to their own files as well,
* either in the `src/utilities` directory if meant to be portable and reusable,
* or in the feature's directory as a .utils.ts file. ex: `SayHello.utils.ts`.
* Isolation makes utils easier to test and reduces the main file size for better readability.
* Doing so also may also reveal the use case is already covered by an existing utility.
*/
export const capitalize = (s: string) => {
return s.charAt(0).toUpperCase() + s.slice(1);
};

export { SayHello };
```

- There are cases where you don't want the prop to be forwarded to the DOM element, so we've provided a helper `isPropValid` to assist in these cases.
- The `label` property in the `styled` API is used to provide a unique identifier for the component when it is being styled. This can be useful when debugging a large codebase, as it can help identify which component the style is being applied to. For example, if you have multiple instances of the `StyledH1` component, the `label` property can help you identify which instance is being styled in the browser's developer tools.

#### Imports

- Import React with `import * as React from 'react'`.
- Use absolute imports, e.g. `import { queryClient } from 'src/queries/base'`.
- Methods from the api-v4 package should be imported from `@linode/api-v4/lib/..`.

#### Composition

When building a large component, it is recommended to break it down and avoid writing several components within the same file. It improves readability and testability. Components should, in most cases, come with their own unit test, although they can be skipped if an e2e suite is covering the functionality.
Utilities should almost always feature a unit test.

#### Styles

- With the transition to MUI v5, the [`styled`](https://mui.com/system/styled/) API, along with the [`sx` prop](https://mui.com/system/getting-started/the-sx-prop/), is the preferred way to specify component-specific styles.
- Component-specific styles may be defined at the end of the component file or in a dedicated file, named `ComponentName.styles.tsx`.
- Component-specific styles may be defined at the end of the component file or in a dedicated file, named `ComponentName.styles.ts`.
- Component files longer than 100 lines must have these styles defined in a dedicated file.

##### Styled Components
- The `label` property in the `styled` API is used to provide a unique identifier for the component when it is being styled. This can be useful when debugging a large codebase, as it can help identify which component the style is being applied to. For example, if you have multiple instances of the `StyledH1` component, the `label` property can help you identify which instance is being styled in the browser's developer tools.
- There are cases where you don't want the prop to be forwarded to the DOM element, so we've provided a helper `omittedProps` to assist in these cases.
It is the responsibility of the developer to check for any console error in case non-semantic props make their way to the dom.

#### Types and Interfaces

- To specify component props, define an interface with the name of the component `MyComponentProps` and pass it to the component as a type argument. This is to provide clarity if ever we need to export this type into another component.
Expand Down
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-9790-changed-1697132255387.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Changed
---

Rename isPropValid utility ([#9790](https://github.com/linode/manager/pull/9790))
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import Popper, { PopperProps } from '@mui/material/Popper';
import { styled } from '@mui/material/styles';
import React from 'react';

import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

export const StyledListItem = styled('li', {
label: 'StyledListItem',
shouldForwardProp: (prop) => isPropValid(['selectAllOption'], prop),
shouldForwardProp: omittedProps(['selectAllOption']),
})(({ theme }) => ({
'&.MuiAutocomplete-option': {
overflow: 'unset',
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/components/BarPercent/BarPercent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { SxProps } from '@mui/system';
import * as React from 'react';

import { LinearProgress } from 'src/components/LinearProgress';
import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

export interface BarPercentProps {
/** Additional css class to pass to the component */
Expand Down Expand Up @@ -69,7 +69,7 @@ const StyledDiv = styled('div')({

const StyledLinearProgress = styled(LinearProgress, {
label: 'StyledLinearProgress',
shouldForwardProp: (prop) => isPropValid(['rounded', 'narrow'], prop),
shouldForwardProp: omittedProps(['rounded', 'narrow']),
})<Partial<BarPercentProps>>(({ theme, ...props }) => ({
'& .MuiLinearProgress-bar2Buffer': {
backgroundColor: '#5ad865',
Expand Down
10 changes: 7 additions & 3 deletions packages/manager/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Reload from 'src/assets/icons/reload.svg';
import { TooltipIcon } from 'src/components/TooltipIcon';

import { rotate360 } from '../../styles/keyframes';
import { isPropValid } from '../../utilities/isPropValid';
import { omittedProps } from '../../utilities/omittedProps';

export type ButtonType = 'outlined' | 'primary' | 'secondary';

Expand Down Expand Up @@ -43,8 +43,12 @@ export interface ButtonProps extends _ButtonProps {
}

const StyledButton = styled(_Button, {
shouldForwardProp: (prop) =>
isPropValid(['compactX', 'compactY', 'loading', 'buttonType'], prop),
shouldForwardProp: omittedProps([
'compactX',
'compactY',
'loading',
'buttonType',
]),
})<ButtonProps>(({ theme, ...props }) => ({
...(props.buttonType === 'secondary' && {
color: theme.textColors.linkActiveLight,
Expand Down
5 changes: 2 additions & 3 deletions packages/manager/src/components/Chip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { default as _Chip, ChipProps as _ChipProps } from '@mui/material/Chip';
import { styled } from '@mui/material/styles';
import * as React from 'react';

import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

export interface ChipProps extends _ChipProps {
/**
Expand Down Expand Up @@ -46,8 +46,7 @@ export const Chip = ({

const StyledChip = styled(_Chip, {
label: 'StyledChip',
shouldForwardProp: (prop) =>
isPropValid(['inTable', 'outlineColor', 'pill'], prop),
shouldForwardProp: omittedProps(['inTable', 'outlineColor', 'pill']),
})<ChipProps>(({ theme, ...props }) => ({
...(props.inTable && {
marginBottom: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
CircularProgress,
CircularProgressProps,
} from 'src/components/CircularProgress';
import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

interface CircleProgressProps extends CircularProgressProps {
children?: JSX.Element;
Expand Down Expand Up @@ -120,7 +120,7 @@ const StyledCircularProgress = styled(CircularProgress)<CircleProgressProps>(
);

const StyledMiniCircularProgress = styled(CircularProgress, {
shouldForwardProp: (prop) => isPropValid(['noPadding'], prop),
shouldForwardProp: omittedProps(['noPadding']),
})<CircleProgressProps>(({ theme, ...props }) => ({
padding: `calc(${theme.spacing()} * 1.3)`,
...(props.noPadding && {
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/components/CopyTooltip/CopyTooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as React from 'react';

import FileCopy from 'src/assets/icons/copy.svg';
import { Tooltip, TooltipProps } from 'src/components/Tooltip';
import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

export interface CopyTooltipProps {
/**
Expand Down Expand Up @@ -73,7 +73,7 @@ export const CopyTooltip = (props: CopyTooltipProps) => {

const StyledCopyTooltipButton = styled('button', {
label: 'StyledCopyTooltipButton',
shouldForwardProp: (prop) => isPropValid(['copyableText', 'text'], prop),
shouldForwardProp: omittedProps(['copyableText', 'text']),
})<Omit<CopyTooltipProps, 'text'>>(({ theme, ...props }) => ({
'& svg': {
color: theme.color.grey1,
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/components/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as React from 'react';
import { Box } from 'src/components/Box';
import { DialogTitle } from 'src/components/DialogTitle/DialogTitle';
import { Notice } from 'src/components/Notice/Notice';
import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';
import { convertForAria } from 'src/utilities/stringUtils';

export interface DialogProps extends _DialogProps {
Expand Down Expand Up @@ -95,7 +95,7 @@ export const Dialog = (props: DialogProps) => {
};

const StyledDialog = styled(_Dialog, {
shouldForwardProp: (prop) => isPropValid(['fullHeight', 'title'], prop),
shouldForwardProp: omittedProps(['fullHeight', 'title']),
})<DialogProps>(({ theme, ...props }) => ({
'& .MuiDialog-paper': {
height: props.fullHeight ? '100vh' : undefined,
Expand Down
10 changes: 7 additions & 3 deletions packages/manager/src/components/Divider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _Divider, { DividerProps as _DividerProps } from '@mui/material/Divider';
import { styled } from '@mui/material/styles';
import * as React from 'react';

import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

export interface DividerProps extends _DividerProps {
dark?: boolean;
Expand All @@ -17,8 +17,12 @@ export const Divider = (props: DividerProps) => {

const StyledDivider = styled(_Divider, {
label: 'StyledDivider',
shouldForwardProp: (prop) =>
isPropValid(['spacingTop', 'spacingBottom', 'light', 'dark'], prop),
shouldForwardProp: omittedProps([
'spacingTop',
'spacingBottom',
'light',
'dark',
]),
})<DividerProps>(({ theme, ...props }) => ({
borderColor: props.dark
? theme.color.border2
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/components/EntityDetail/EntityDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Grid from '@mui/material/Unstable_Grid2';
import { styled } from '@mui/material/styles';
import * as React from 'react';

import { isPropValid } from '../../utilities/isPropValid';
import { omittedProps } from '../../utilities/omittedProps';

export interface EntityDetailProps {
body?: JSX.Element;
Expand Down Expand Up @@ -42,7 +42,7 @@ const GridBody = styled(Grid)(({ theme }) => ({

const GridFooter = styled(Grid, {
label: 'EntityDetailGridFooter',
shouldForwardProp: (prop) => isPropValid(['body'], prop),
shouldForwardProp: omittedProps(['body']),
})<Partial<EntityDetailProps>>(({ theme, ...props }) => ({
alignItems: 'center',
backgroundColor: theme.bg.bgPaper,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { styled } from '@mui/material/styles';

import { Button } from 'src/components/Button/Button';
import { Typography } from 'src/components/Typography';
import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

interface DropZoneClassProps {
dropzoneDisabled: boolean;
Expand All @@ -13,11 +13,12 @@ interface DropZoneClassProps {

export const StyledDropZoneDiv = styled('div', {
label: 'StyledDropZoneDiv',
shouldForwardProp: (prop) =>
isPropValid(
['dropzoneDisabled', 'isDragActive', 'isDragAccept', 'isDragReject'],
prop
),
shouldForwardProp: omittedProps([
'dropzoneDisabled',
'isDragActive',
'isDragAccept',
'isDragReject',
]),
})<DropZoneClassProps>(({ theme, ...props }) => ({
backgroundColor: 'transparent',
borderColor: theme.palette.primary.main,
Expand Down Expand Up @@ -68,7 +69,7 @@ export const StyledFileUploadsDiv = styled('div', {

export const StyledDropZoneContentDiv = styled('div', {
label: 'StyledDropZoneContentDiv',
shouldForwardProp: (prop) => isPropValid(['uploadZoneActive'], prop),
shouldForwardProp: omittedProps(['uploadZoneActive']),
})<{
uploadZoneActive: boolean;
}>(({ theme, ...props }) => ({
Expand Down
32 changes: 17 additions & 15 deletions packages/manager/src/components/GaugePercent/GaugePercent.styles.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,40 @@
import { styled } from '@mui/material/styles';
import { isPropValid } from 'src/utilities/isPropValid';

import { omittedProps } from 'src/utilities/omittedProps';

import { GaugePercentProps } from './GaugePercent';

type StyledGaugePercentProps = Pick<GaugePercentProps, 'innerTextFontSize'> &
Required<Pick<GaugePercentProps, 'width' | 'height'>>;
Required<Pick<GaugePercentProps, 'height' | 'width'>>;

const propKeys = ['width', 'height', 'innerTextFontSize'];

export const StyledSubTitleDiv = styled('div', {
shouldForwardProp: (prop) => isPropValid(propKeys, prop),
})<StyledGaugePercentProps>(({ theme, width, height, innerTextFontSize }) => ({
shouldForwardProp: omittedProps(propKeys),
})<StyledGaugePercentProps>(({ height, innerTextFontSize, theme, width }) => ({
color: theme.color.headline,
fontSize: innerTextFontSize || theme.spacing(2.5),
position: 'absolute',
textAlign: 'center',
top: `calc(${height}px + ${theme.spacing(1.25)})`,
width: width,
width,
}));

export const StyledInnerTextDiv = styled('div', {
shouldForwardProp: (prop) => isPropValid(propKeys, prop),
})<StyledGaugePercentProps>(({ theme, height, width }) => ({
shouldForwardProp: omittedProps(propKeys),
})<StyledGaugePercentProps>(({ height, theme, width }) => ({
color: theme.palette.text.primary,
fontSize: '1rem',
position: 'absolute',
top: `calc(${height + 30}px / 2)`,
width: width,
textAlign: 'center',
fontSize: '1rem',
color: theme.palette.text.primary,
top: `calc(${height + 30}px / 2)`,
width,
}));

export const StyledGaugeWrapperDiv = styled('div', {
shouldForwardProp: (prop) => isPropValid(propKeys, prop),
})<StyledGaugePercentProps>(({ theme, height, width }) => ({
position: 'relative',
width: width,
shouldForwardProp: omittedProps(propKeys),
})<StyledGaugePercentProps>(({ height, theme, width }) => ({
height: `calc(${height}px + ${theme.spacing(3.75)})`,
position: 'relative',
width,
}));
4 changes: 2 additions & 2 deletions packages/manager/src/components/LineGraph/LineGraph.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Table } from 'src/components/Table';
import { TableBody } from 'src/components/TableBody';
import { TableCell } from 'src/components/TableCell';
import { TableHead } from 'src/components/TableHead';
import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

export const StyledWrapper = styled('div')(() => ({
display: 'flex',
Expand Down Expand Up @@ -131,7 +131,7 @@ export const StyledButton = styled(Button, {

export const StyledButtonElement = styled('span', {
label: 'StyledButtonElement',
shouldForwardProp: (prop) => isPropValid(['hidden', 'sx'], prop),
shouldForwardProp: omittedProps(['hidden', 'sx']),
})(({ ...props }) => ({
...(props.hidden && {
textDecoration: 'line-through',
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/components/Paper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _Paper, { PaperProps } from '@mui/material/Paper';
import { styled } from '@mui/material/styles';
import * as React from 'react';

import { isPropValid } from 'src/utilities/isPropValid';
import { omittedProps } from 'src/utilities/omittedProps';

import { FormHelperText } from './FormHelperText';

Expand Down Expand Up @@ -37,7 +37,7 @@ export const Paper = (props: Props) => {
};

const StyledPaper = styled(_Paper, {
shouldForwardProp: (prop) => isPropValid(['error'], prop),
shouldForwardProp: omittedProps(['error']),
})<Props>(({ theme, ...props }) => ({
borderColor: props.error ? theme.color.red : undefined,
padding: theme.spacing(3),
Expand Down
Loading

0 comments on commit d542062

Please sign in to comment.