✨ Add option to upload background image #15
Conversation
Using react-dropzone and FileReader to allow user to upload image from computer onto background-image of the banner christopherkade#5
|
Hi @christopherkade have done the basics for upload images, needs a bit of tweaking, but thought its good to get some feedback at this point. Also the checks didnt work as you can see - the error says to run yarn add react-dropzone, is that correct? |
|
This is awesome work, I've been fiddling around with it and you did a great job. I've done a code review, most of these changes are tiny so don't worry if you see so many comments ! |
Add react-dropzone dep, removed unused props #15
| padding: 20px; | ||
| ` | ||
|
|
||
| const ImageInput = ({ image, setImage }) => { |
| const ImageInput = ({ image, setImage }) => { | ||
| const onDrop = useCallback(acceptedFiles => { | ||
| const file = acceptedFiles[0] | ||
| let reader = new FileReader(); |
There was a problem hiding this comment.
Indentation + declare reader as a constant
| <> | ||
| <InputBlock> | ||
| <div {...getRootProps({className: 'dropzone'})}> | ||
| <input {...getInputProps()} /> |
There was a problem hiding this comment.
Indentation (you can activate automatic formatting in VS Code in your settings, that will normally apply our eslint formatting to your whole file on each save)
There was a problem hiding this comment.
Thanks for the tip :) I havent been using VS Code but am trying it now - I added the Eslint plugin and checked 'Turns auto fix on save on or off' in settings, but it doesnt seem to fix the formatting on save - have I missed something?
| <InputBlock> | ||
| <div {...getRootProps({className: 'dropzone'})}> | ||
| <input {...getInputProps()} /> | ||
| <p>Drag 'n' drop some files here, or click to select files</p> |
There was a problem hiding this comment.
What do you think of Chose a file or drag it here ?
There was a problem hiding this comment.
Yes sounds better
| @@ -35,8 +37,8 @@ const Summary = styled.summary` | |||
| ` | |||
|
|
|||
| const InputWrapper = ({ values, setters }) => { | |||
| const { bgColor, title, titleSize, titleColor, borderColor, borderSize } = values | |||
| const { setBgColor, setTitle, setTitleSize, setTitleColor, setBorderColor, setBorderSize } = setters | |||
| const { bgColor, title, titleSize, titleColor, borderColor, borderSize, image } = values | |||
| @@ -56,6 +58,10 @@ const InputWrapper = ({ values, setters }) => { | |||
| <Summary>Sizes</Summary> | |||
| <SizeInput titleSize={titleSize} borderSize={borderSize} setTitleSize={setTitleSize} setBorderSize={setBorderSize} /> | |||
| </Detail> | |||
| <Detail> | |||
| <Summary>Background Image</Summary> | |||
| <ImageInput image={image} setImage={setImage}/> | |||
There was a problem hiding this comment.
Since image is not used in this component, remove it as a prop
| @@ -13,6 +13,7 @@ const Wrapper = styled.div` | |||
| border: 1px solid #dbdbdb; | |||
| border: ${(props) => `${props.borderSize}px solid ${props.borderColor}`}; | |||
| box-sizing: border-box; | |||
| background-image: url(${(props) => props.image}); | |||
There was a problem hiding this comment.
What do you think of adding:
background-repeat: no-repeat;
background-size: cover;?
|
Thanks for the feedback, I am going through these. Im happy about the many comments, its good to learn! :D |
Using react-dropzone and FileReader to allow user to upload image from computer onto background-image of the banner #5