|
| 1 | +# 09 Colorpicker Refactor |
| 2 | + |
| 3 | +In this example we are going to review the colorpicker component we have created and simplify it. Right now we have three slider controls with many details that make our HTML hard to read. Let's componentize this scenario. |
| 4 | + |
| 5 | +We take _07 Colorpicker_ as reference. |
| 6 | + |
| 7 | +# Steps to reproduce this sample |
| 8 | + |
| 9 | +- Copy the content from _07 Colorpicker_ and execute _npm install_. |
| 10 | + |
| 11 | +```bash |
| 12 | +npm install |
| 13 | +``` |
| 14 | + |
| 15 | +- In the previous example we were copying and pasting + toggling property names on the following piece of code. |
| 16 | + |
| 17 | +_DO NOT COPY THIS_ |
| 18 | + |
| 19 | +```typescript |
| 20 | +<input |
| 21 | + type="range" |
| 22 | + min="0" |
| 23 | + max="255" |
| 24 | + value={props.color.red} |
| 25 | + onChange={event => |
| 26 | + props.onColorUpdated({ |
| 27 | + red: +event.target.value, |
| 28 | + green: props.color.green, |
| 29 | + blue: props.color.blue |
| 30 | + }) |
| 31 | + } |
| 32 | +/>; |
| 33 | +{ |
| 34 | + props.color.red; |
| 35 | +} |
| 36 | +``` |
| 37 | + |
| 38 | +- We will start our refactor by componentizing this piece of code, |
| 39 | + let's append to the _colorPicker.tsx_ the following component |
| 40 | + (another approach could be to create a new file, that's a tetchty decisition, |
| 41 | + on one hand it makes sense to keep in the same file the slider since is quite |
| 42 | + related to the picker, on the other segregating in diferent files is another |
| 43 | + valid approach). |
| 44 | + |
| 45 | +_./src/components/colorPicker.tsx_ |
| 46 | + |
| 47 | +**Append this to the colorPicker file** |
| 48 | + |
| 49 | +```typescript |
| 50 | +interface PropsColorSlider { |
| 51 | + value: number; |
| 52 | + onValueUpdated: (newValue: number) => void; |
| 53 | +} |
| 54 | + |
| 55 | +const ColorSliderComponent = (props: PropsColorSlider) => { |
| 56 | + return ( |
| 57 | + <div> |
| 58 | + <input |
| 59 | + type="range" |
| 60 | + min="0" |
| 61 | + max="255" |
| 62 | + value={props.value} |
| 63 | + onChange={event => props.onValueUpdated(+event.target.value)} |
| 64 | + /> |
| 65 | + {props.value} |
| 66 | + </div> |
| 67 | + ); |
| 68 | +}; |
| 69 | +``` |
| 70 | + |
| 71 | +- Now we can simplify our ColorPicker code (first iteration...): |
| 72 | + |
| 73 | +_./src/component/colorPicker.tsx_ |
| 74 | + |
| 75 | +```diff |
| 76 | +export const ColorPicker = (props: Props) => ( |
| 77 | + <div> |
| 78 | +- <input |
| 79 | +- type="range" |
| 80 | +- min="0" |
| 81 | +- max="255" |
| 82 | +- value={props.color.red} |
| 83 | +- onChange={event => |
| 84 | +- props.onColorUpdated({ |
| 85 | +- red: +event.target.value, |
| 86 | +- green: props.color.green, |
| 87 | +- blue: props.color.blue |
| 88 | +- }) |
| 89 | +- } |
| 90 | +- /> |
| 91 | +- {props.color.red} |
| 92 | ++ <ColorSliderComponent |
| 93 | ++ value = {props.color.red} |
| 94 | ++ onValueUpdated={(value) => props.onColorUpdated( |
| 95 | ++ { |
| 96 | ++ red: value, |
| 97 | ++ green: props.color.green, |
| 98 | ++ blue: props.color.blue |
| 99 | ++ }) |
| 100 | ++ } |
| 101 | ++ /> |
| 102 | + <br /> |
| 103 | +- <input |
| 104 | +- type="range" |
| 105 | +- min="0" |
| 106 | +- max="255" |
| 107 | +- value={props.color.green} |
| 108 | +- onChange={(event: any) => |
| 109 | +- props.onColorUpdated({ |
| 110 | +- red: props.color.red, |
| 111 | +- green: event.target.value, |
| 112 | +- blue: props.color.blue |
| 113 | +- }) |
| 114 | +- } |
| 115 | +- /> |
| 116 | +- {props.color.green} |
| 117 | ++ <ColorSliderComponent |
| 118 | ++ value = {props.color.green} |
| 119 | ++ onValueUpdated={(value) => props.onColorUpdated( |
| 120 | ++ { |
| 121 | ++ red: props.color.red, |
| 122 | ++ green: value, |
| 123 | ++ blue: props.color.blue |
| 124 | ++ }) |
| 125 | ++ } |
| 126 | ++ /> |
| 127 | + <br /> |
| 128 | +- <input |
| 129 | +- type="range" |
| 130 | +- min="0" |
| 131 | +- max="255" |
| 132 | +- value={props.color.blue} |
| 133 | +- onChange={(event: any) => |
| 134 | +- props.onColorUpdated({ |
| 135 | +- red: props.color.red, |
| 136 | +- green: props.color.green, |
| 137 | +- blue: event.target.value |
| 138 | +- }) |
| 139 | +- } |
| 140 | +- /> |
| 141 | +- {props.color.blue} |
| 142 | ++ <ColorSliderComponent |
| 143 | ++ value = {props.color.blue} |
| 144 | ++ onValueUpdated={(value) => props.onColorUpdated( |
| 145 | ++ { |
| 146 | ++ red: props.color.red, |
| 147 | ++ green: props.color.green, |
| 148 | ++ blue: value |
| 149 | ++ }) |
| 150 | ++ } |
| 151 | ++ /> |
| 152 | + <br /> |
| 153 | + </div> |
| 154 | +); |
| 155 | +``` |
| 156 | + |
| 157 | +- Let's check that we haven't broken anything. |
| 158 | + |
| 159 | +```bash |
| 160 | +npm start |
| 161 | +``` |
| 162 | + |
| 163 | +- Not bad, we saved some lines of code and by adding using _ColorSliderComponent_ it's |
| 164 | + easier to read the content of the \_ColorPicker. Let's keep on refactoring the code. |
| 165 | + |
| 166 | +- Second iteration: We have still room for improvement. What about using a single handler for all colors? If we currify the colorupdated handler, then we can! |
| 167 | + |
| 168 | +_./src/component/colorPicker.tsx_ |
| 169 | + |
| 170 | +```diff |
| 171 | ++ const updateColor = (props : Props, colorId : keyof Color) => (value) => { // keyof Color ensures only 'red', 'blue' or 'green' can be passed in. |
| 172 | ++ props.onColorUpdated({ |
| 173 | ++ ...props.color, // this creates a clone of the current props.color object... |
| 174 | ++ [colorId]: value // ... which gets one of its properties (colorId) immediately replaced by a new value. |
| 175 | ++ }); |
| 176 | ++ }; |
| 177 | + |
| 178 | +export const ColorPicker = (props: Props) => { |
| 179 | + return ( |
| 180 | + <div> |
| 181 | + <ColorSliderComponent |
| 182 | + value = {props.color.red} |
| 183 | ++ onValueUpdated={updateColor(props, 'red')} |
| 184 | +- onValueUpdated={(value) => props.onColorUpdated( |
| 185 | +- { |
| 186 | +- red: value, |
| 187 | +- green: props.color.green, |
| 188 | +- blue: props.color.blue |
| 189 | +- }) |
| 190 | +- } |
| 191 | + /> |
| 192 | + <br /> |
| 193 | + <ColorSliderComponent |
| 194 | + value = {props.color.green} |
| 195 | ++ onValueUpdated={updateColor(props, 'green')} |
| 196 | +- onValueUpdated={(value) => props.onColorUpdated( |
| 197 | +- { |
| 198 | +- red: props.color.red, |
| 199 | +- green: value, |
| 200 | +- blue: props.color.blue |
| 201 | +- }) |
| 202 | +- } |
| 203 | + /> |
| 204 | + <br /> |
| 205 | + <ColorSliderComponent |
| 206 | + value = {props.color.blue} |
| 207 | ++ onValueUpdated={updateColor(props, 'blue')} |
| 208 | +- onValueUpdated={(value) => props.onColorUpdated( |
| 209 | +- { |
| 210 | +- red: props.color.red, |
| 211 | +- green: props.color.green, |
| 212 | +- blue: value |
| 213 | +- }) |
| 214 | +- } |
| 215 | + /> |
| 216 | + <br /> |
| 217 | + </div> |
| 218 | + ); |
| 219 | +} |
| 220 | +``` |
| 221 | + |
| 222 | +- Now we got a great result !! we have enhanced code quality in our component. |
| 223 | + |
| 224 | +```bash |
| 225 | +npm start |
| 226 | +``` |
| 227 | + |
| 228 | +- Could we go one step furhter refactoring? The answer is yes, could it be worth? That's |
| 229 | + worth a discussion, sometimes is a good idea to keep on refactoring, and then rollback |
| 230 | + one step, let's apply the following trick, let's replace our color picker with this code: |
| 231 | + |
| 232 | +_./src/component/colorPicker.tsx_ |
| 233 | + |
| 234 | +```typescript |
| 235 | +export const ColorPicker = (props: Props) => ( |
| 236 | + <> |
| 237 | + {Object.keys(props.color).map((field: keyof Color) => ( |
| 238 | + <ColorSliderComponent |
| 239 | + key={field} |
| 240 | + value={props.color[field]} |
| 241 | + onValueUpdated={updateColor(props, field)} |
| 242 | + /> |
| 243 | + ))} |
| 244 | + </> |
| 245 | +); |
| 246 | +``` |
| 247 | + |
| 248 | +> Excercise: evaluate what this code does, is this code worth? what issues could you find |
| 249 | +> in the future? What would happend if we add new fields to the color entity? |
0 commit comments