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.

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: Percentage field color customization #692

Merged
merged 30 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8644367
Merge pull request #685 from rowyio/develop
shamsmosowi Jun 16, 2022
40e259e
Merge branch 'develop' into rc
notsidney Jun 16, 2022
ad586e7
Merge branch 'develop' into rc
shamsmosowi Jun 17, 2022
57e758d
Merge pull request #686 from rowyio/rc
shamsmosowi Jun 17, 2022
f9e5be7
feat(percentage-c11n): convert to table cell
Jun 22, 2022
4882048
feat(percentage-c11n): add logic to default configs
Jun 22, 2022
f8c4269
feat(percentage-c11n): add color picker to settings
Jun 22, 2022
976dd7f
feat(percentage-c11n): change default colors
Jun 22, 2022
9bd1a4c
feat(percentage-c11n): fix button text color
Jun 22, 2022
52964a8
feat(percentage-c11n): add labels to settings
Jun 22, 2022
d74ac13
feat(percentage-c11n): add preview section
Jun 22, 2022
8854965
feat(percentage-c11n): fix cache issues with debouncing
Jun 23, 2022
13db993
feat(percentage-c11n): add width responsiveness to color picker
Jun 23, 2022
c25f420
feat(percentage-c11n): fix responsiveness issues
Jun 25, 2022
5afd5c7
feat(percentage-c11n): add checkbox, refactor a little
Jun 25, 2022
7ca47fa
feat(percentage-c11n): convert data type to array
Jun 28, 2022
591ccb2
feat(percentage-c11n): refactor config states
Jun 29, 2022
5069a75
feat(percentage-c11n): fix defaults
Jun 29, 2022
531b996
feat(percentage-c11n): add basic cell without bg
Jun 29, 2022
aa8d086
feat(percentage-c11n): remove collapse
Jun 29, 2022
5161c7c
feat(percentage-c11n): refactor checkStates
Jun 29, 2022
8fac4b2
feat(percentage-c11n): add grid layout
Jun 29, 2022
21443cd
feat(percentage-c11n): chore conventions
Jun 30, 2022
9fd88b3
feat(percentage-c11n): add default theme color to sidedrawer
Jun 30, 2022
2c1fbca
remove redundant fragment
htuerker Jun 30, 2022
bb73673
fix text color in preview
htuerker Jun 30, 2022
18aa406
fix: change state to derived state
htuerker Jul 2, 2022
e2bfef2
fix: review suggestions
Jul 2, 2022
0bd6f63
fix: remove redundant change call
htuerker Jul 4, 2022
d015144
fix(percentage-c11n): remove redundant dependencies
Jul 4, 2022
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
92 changes: 92 additions & 0 deletions src/components/ColorPickerInput.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import {
useState,
useEffect,
useRef,
MutableRefObject,
useLayoutEffect,
} from "react";
import { Box } from "@mui/material";

import { fieldSx } from "@src/components/SideDrawer/utils";
import { Color, ColorPicker } from "react-color-palette";
import { useDebouncedCallback } from "use-debounce";

const useResponsiveWidth = (): [
width: number,
setRef: MutableRefObject<HTMLElement | null>
] => {
const ref = useRef(null);
const [width, setWidth] = useState(0);

useLayoutEffect(() => {
if (!ref || !ref.current) {
return;
}
const resizeObserver = new ResizeObserver((targets) => {
const { width: currentWidth } = targets[0].contentRect;
setWidth(currentWidth);
});

resizeObserver.observe(ref.current);

return () => {
resizeObserver.disconnect();
};
}, []);

return [width, ref];
};

export interface IColorPickerProps {
value: Color;
handleOnChangeComplete: (color: Color) => void;
disabled?: boolean;
}

export default function ColorPickerInput({
value,
handleOnChangeComplete,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
handleOnChangeComplete: (color: Color) => void;
disabled?: boolean;
}
export default function ColorPickerInput({
value,
handleOnChangeComplete,
onChangeComplete: ColorPickerProps['onChangeComplete'];
disabled?: boolean;
}
export default function ColorPickerInput({
value,
onChangeComplete,

Pass the onChangeComplete prop to the <ColorPicker> component directly. You also need to import ColorPickerProps from react-color-palette.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a nice trick I did not know it, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better not to use this as onChangeComplete is optional in ColorPickerProps.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better not to use this as onChangeComplete is optional in ColorPickerProps.

You can use NonNullable<ColorPickerProps['onChangeComplete']> (TypeScript docs)

disabled = false,
}: IColorPickerProps) {
const [localValue, setLocalValue] = useState(value);
const [width, setRef] = useResponsiveWidth();

const debouncedOnChangeComplete = useDebouncedCallback((color) => {
handleOnChangeComplete(color);
}, 100);

useEffect(() => {
debouncedOnChangeComplete(localValue);
}, [debouncedOnChangeComplete, localValue]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m not sure why this is done this way. react-color-picker already has a onChangeComplete prop fired on mouseup. Then you can avoid debouncing and using an effect.

If you need to debounce (when writing to db), it should be handled in the parent component. If the reason for debounce is to improve performance, you can use React 18’s startTransition

Suggested change
const debouncedOnChangeComplete = useDebouncedCallback((color) => {
handleOnChangeComplete(color);
}, 100);
useEffect(() => {
debouncedOnChangeComplete(localValue);
}, [debouncedOnChangeComplete, localValue]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem here is every color changes frame(100 times+ just for a little drag) appends lots of styles into head as we're generating styles regarding. I'm aware of this problem for now. This also makes hard to reuse this component I already tried it for Color field.

I'm actually planning to work on another related issue after this. I've decided to think through this reusability problems during slider customization implementation. I'll research how we can use startTransition btw.

Also, a similar implementation used in Color field's PopoverCell.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem here is every color changes frame(100 times+ just for a little drag)

It only re-renders this ColorPickerInput component, which is expected and necessary. This is why we use onChangeComplete to save the selected color only when the user has stopped interacting with the picker, and then update the other components like preview.

appends lots of styles into head as we're generating styles regarding.

This only happens when the user stops dragging (onChangeComplete), and MUI only appends styles to head in development mode. Notice that in dev mode, there are 1000+ nodes of <style data-emotion="mui" data-s> but these do not appear in production (like demo.rowy.io). So this isn’t a problem.

Also, a similar implementation used in Color field's PopoverCell.

This is because when onChangeComplete is called there, it immediately writes to the database, and we need to debounce writes for that since the user pays for writes. In this case, it’s all local, so we don’t have to add a debounce.


Additionally, now that you’ve accepted the suggestion to pass onChangeComplete directly, you’ve made this debounce + effect redundant. Adding logs, I saw that:

  1. It fired when the dropdown is first opened.
  2. It fired when the user stopped moving the mouse, but is still holding down the mouse button.
  3. It fired when the user stopped holding the mouse button (this is the only time we want to fire), but since we already have an onChangeComplete prop, onChangeComplete is called twice at this point.

I’ve added another change request and when you click accept on that in GitHub, I’ll merge this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is obviously better!


return (
<Box
ref={setRef}
sx={[
fieldSx,
{
marginTop: "1rem",
padding: "1rem",
borderColor: "divider",
transitionDuration: 0,
"& .rcp": {
border: "none",
"& .rcp-saturation": {
borderRadius: "4px",
},
"& .rcp-body": {
boxSizing: "unset",
},
},
},
]}
>
<ColorPicker
width={width}
height={100}
color={localValue}
onChange={(color) => setLocalValue(color)}
/>
</Box>
);
}
41 changes: 0 additions & 41 deletions src/components/fields/Percentage/BasicCell.tsx

This file was deleted.

195 changes: 195 additions & 0 deletions src/components/fields/Percentage/Settings.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
import { useState } from "react";

import {
Box,
ButtonBase,
Checkbox,
Collapse,
InputLabel,
useTheme,
} from "@mui/material";
import ColorPickerInput from "@src/components/ColorPickerInput";
import { ISettingsProps } from "@src/components/fields/types";

import { Color, toColor } from "react-color-palette";
import { fieldSx } from "@src/components/SideDrawer/utils";
import { ChevronDown } from "mdi-material-ui";
import { ReactElement } from "react-markdown/lib/react-markdown";
import { resultColorsScale } from "@src/utils/color";

const ColorPickerCollapse = ({
colorKey,
color,
active,
setActive,
disabled,
children,
}: {
colorKey: string;
color: Color;
active: boolean;
setActive: (activeKey: string | null) => void;
disabled?: boolean;
children?: ReactElement;
}) => {
const toggleCollapse = () => !disabled && setActive(active ? null : colorKey);

const toggleState = (() => {
if (disabled && active) {
setActive(null);
return false;
}
return active;
})();

return (
<Box sx={{ width: "100%" }}>
<ButtonBase
onClick={toggleCollapse}
component={ButtonBase}
focusRipple
disabled={disabled}
sx={[
fieldSx,
{
justifyContent: "flex-start",
"&&": { pl: 0.75, pr: 0.5 },
color: color.hex,
transition: (theme) =>
theme.transitions.create("border-radius", {
delay: theme.transitions.duration.standard,
}),
"&.Mui-disabled": { color: "text.disabled" },
},
active && {
transitionDelay: "0s",
transitionDuration: "0s",
},
]}
>
<Box
sx={{
backgroundColor: color?.hex,
width: 15,
height: 15,
mr: 1.5,
boxShadow: (theme) => `0 0 0 1px ${theme.palette.divider} inset`,
borderRadius: 0.5,
opacity: 0.5,
}}
/>
<div style={{ flexGrow: 1 }}>{color.hex}</div>
<ChevronDown
color="action"
sx={{
transition: (theme) => theme.transitions.create("transform"),
transform: active ? "rotate(180deg)" : "none",
}}
/>
</ButtonBase>
<Collapse in={toggleState}>{children}</Collapse>
</Box>
);
};

const defaultColors: { [key: string]: Color } = {
default: toColor("hex", "#FFFFFF"),
startColor: toColor("hex", "#ED4747"),
midColor: toColor("hex", "#F3C900"),
endColor: toColor("hex", "#1FAD5F"),
};

const colorLabels: { [key: string]: string } = {
startColor: "Start Color",
midColor: "Middle Color",
endColor: "End Color",
};

export default function Settings({ onChange, config }: ISettingsProps) {
const [checkStates, setCheckStates] = useState<{ [key: string]: boolean }>({
startColor: Boolean(config.startColor),
midColor: Boolean(config.midColor),
endColor: Boolean(config.endColor),
});
const [activePicker, setActivePicker] = useState<string | null>(null);
const theme = useTheme();

const onCheckboxChange = (key: string, checked: boolean) => {
if (!checked) {
onChange(key)(defaultColors.default);
} else {
onChange(key)(defaultColors[key]);
}
setCheckStates({ ...checkStates, [key]: checked });
};

return (
<>
{Object.keys(checkStates).map((colorKey) => {
const color = config[colorKey] || defaultColors[colorKey];
return (
<Box key={colorKey} sx={{ display: "flex", flexDirection: "column" }}>
{colorLabels[colorKey]}
<Box
sx={{
display: "flex",
alignItems: "start",
justifyContent: "center",
}}
>
<Checkbox
sx={[
fieldSx,
{ width: "auto", marginRight: "0.5rem", boxShadow: "none" },
]}
checked={checkStates[colorKey]}
onChange={() =>
onCheckboxChange(colorKey, !checkStates[colorKey])
}
/>
<ColorPickerCollapse
colorKey={colorKey}
active={activePicker === colorKey}
setActive={(activePicker) => setActivePicker(activePicker)}
color={color}
disabled={!checkStates[colorKey]}
>
<ColorPickerInput
value={color}
handleOnChangeComplete={(color) => onChange(colorKey)(color)}
disabled={!checkStates[colorKey]}
/>
</ColorPickerCollapse>
</Box>
</Box>
);
})}
<InputLabel>
Preview:
<Box sx={{ display: "flex", textAlign: "center" }}>
{[0, 0.125, 0.25, 0.375, 0.5, 0.625, 0.75, 0.875, 1].map((value) => {
const { startColor, midColor, endColor } = config;
return (
<Box
key={value}
sx={{
width: "100%",
padding: "0.5rem 0",
color: theme.palette.text.primary,
backgroundColor: resultColorsScale(value, {
startColor: startColor.hex,
midColor: midColor.hex,
endColor: endColor.hex,
}).toHex(),
opacity: 0.5,
}}
>
{Math.floor(value * 100)}%
</Box>
);
})}
</Box>
</InputLabel>
</>
);
}
26 changes: 25 additions & 1 deletion src/components/fields/Percentage/SideDrawerField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { TextField, InputAdornment, Box } from "@mui/material";
import { emphasize } from "@mui/material/styles";
import { resultColorsScale } from "@src/utils/color";
import { getFieldId } from "@src/components/SideDrawer/utils";
import { Color, toColor } from "react-color-palette";

export default function Percentage({
column,
Expand All @@ -12,6 +13,29 @@ export default function Percentage({
onSubmit,
disabled,
}: ISideDrawerFieldProps) {
const defaultColors = {
startColor: toColor("hex", "#ED4747"),
midColor: toColor("hex", "#F3C900"),
endColor: toColor("hex", "#1FAD5F"),
};
const {
startColor,
midColor,
endColor,
}: {
startColor: Color;
endColor: Color;
midColor: Color;
} = {
...defaultColors,
...(column as any).config,
};

const colors = {
startColor: startColor.hex,
midColor: midColor.hex,
endColor: endColor.hex,
};
return (
<TextField
variant="filled"
Expand All @@ -38,7 +62,7 @@ export default function Percentage({
`0 0 0 1px ${theme.palette.divider} inest`,
backgroundColor:
typeof value === "number"
? resultColorsScale(value).toHex() + "!important"
? resultColorsScale(value, colors).toHex() + "!important"
: undefined,
}}
/>
Expand Down
Loading