feat(overlays): MIN-192 add overlay as user
Short description:
Add overlay as user
Approach:
Added the ability to create a user overlay by uploading a file or entering a list of elements. Added automatic change of overlay name and description.
Merge request reports
Activity
assigned to @mateusz-winiarczyk
17 OVERLAY_TYPES, 18 } from './UserOverlayForm.constants'; 19 import { FileUpload } from './FileUpload'; 20 import { parseOverlay } from './UserOverlayForm.utils'; 21 22 export const UserOverlayForm = (): React.ReactNode => { 23 const dispatch = useAppDispatch(); 24 const [name, setName] = useState(''); 25 const [type, setType] = useState<SelectorItem>(DEFAULT_TYPE); 26 const [group, setGroup] = useState<SelectorItem>(DEFAULT_GROUP); 27 const [description, setDescription] = useState(''); 28 const [uploadedFile, setUploadedFile] = useState<File | null>(null); 29 const [elementsList, setElementsList] = useState(''); 30 const [overlayContent, setOverlayContent] = useState(''); 31 const projectId = useAppSelector(projectIdSelector); 32 const loadingAddOverlayStatus = useAppSelector(loadingAddOverlay); - Comment on lines +24 to +32
IMO that's a problem-generating solution. It's hardly readable, it's not easy to cover every case with tests and it requires long or unnecessary complex logic.
RFC: please use some form logic provider like https://react-hook-form.com/ or Formik
changed this line in version 2 of the diff
48 filename, 49 name, 50 projectId, 51 type: type.id, 52 }), 53 ); 54 55 setName(''); 56 setDescription(''); 57 setElementsList(''); 58 setOverlayContent(''); 59 setUploadedFile(null); 60 }; 61 62 const navigateToOverlays = (): void => { 63 dispatch(openOverlaysDrawer()); changed this line in version 2 of the diff
63 dispatch(openOverlaysDrawer()); 64 }; 65 66 const handleChangeName = (e: ChangeEvent<HTMLInputElement>): void => { 67 setName(e.target.value); 68 }; 69 const handleChangeDescription = (e: ChangeEvent<HTMLTextAreaElement>): void => { 70 setDescription(e.target.value); 71 }; 72 73 const handleOverlayChange = (nameType: string, value: string): void => { 74 if (nameType === 'NAME') { 75 setName(value); 76 } else if (nameType === 'DESCRIPTION') { 77 setDescription(value); 78 } else if (nameType === 'TYPE') { changed this line in version 2 of the diff
124 className="mt-2.5 w-full resize-none rounded-lg border border-transparent bg-cultured px-2 py-2.5 text-xs font-semibold outline-none hover:border-greyscale-600 focus:border-greyscale-600" 125 /> 126 </label> 127 </div> 128 129 <label className="mb-2.5 text-sm" htmlFor="name"> 130 Name 131 <input 132 type="text" 133 name="name" 134 id="name" 135 data-testid="overlay-name" 136 value={name} 137 onChange={handleChangeName} 138 placeholder="Overlays 11/07/2022" 139 className="mt-2.5 h-12 w-full rounded-lg border border-transparent bg-cultured px-2 py-2.5 text-xs font-semibold outline-none hover:border-greyscale-600 focus:border-greyscale-600" changed this line in version 2 of the diff
1 /* eslint-disable no-magic-numbers */ 2 3 type OverlayDataCallback = { 4 (nameType: string, value: string): void; 5 }; 6 7 export const parseOverlay = (fileContent: string, callback: OverlayDataCallback): void => { changed this line in version 2 of the diff
2 3 type OverlayDataCallback = { 4 (nameType: string, value: string): void; 5 }; 6 7 export const parseOverlay = (fileContent: string, callback: OverlayDataCallback): void => { 8 const content = fileContent.trim(); 9 const lines = content.split('\n'); 10 11 lines.forEach(line => { 12 if (line.indexOf('#') === 0) { 13 if (line.indexOf('=') > 0) { 14 const nameType = line.substring(1, line.indexOf('=')).trim(); 15 const value = line.substring(line.indexOf('=') + 1).trim(); 16 callback(nameType, value); 17 } changed this line in version 2 of the diff
23 28 <p className="mb-5 text-sm"> 24 29 You are not logged in, please login to upload and view custom overlays 25 30 </p> 26 <Button onClick={handleLoginClick}>Login</Button> 31 <Button onClick={handleLoginClick} aria-label="login button"> 32 Login 33 </Button> 27 34 </> 28 35 )} 29 36 30 37 {/* TODO: Implement user overlays */} changed this line in version 2 of the diff
55 createdFile: CreatedOverlayFile; 56 overlayContent: string; 57 }; 58 const uploadContent = async ({ createdFile, overlayContent }: UploadContentArgs): Promise<void> => { 59 const data = new Uint8Array(new TextEncoder().encode(overlayContent)); 60 let uploadedLength = 0; 61 62 const sendChunk = async (): Promise<unknown> => { 63 if (uploadedLength >= data.length) { 64 return; 65 } 66 67 const chunk = data.slice(uploadedLength, uploadedLength + CHUNK_SIZE); 68 69 const responeJSON = await fetch( 70 `https://lux1.atcomp.pl/minerva/api/${apiPath.uploadOverlayFileContent(createdFile.id)}`, changed this line in version 2 of the diff
47 }, 48 ); 49 50 const isDataValid = validateDataUsingZodSchema(response.data, createdOverlayFileSchema); 51 return isDataValid ? response.data : undefined; 52 }; 53 54 type UploadContentArgs = { 55 createdFile: CreatedOverlayFile; 56 overlayContent: string; 57 }; 58 const uploadContent = async ({ createdFile, overlayContent }: UploadContentArgs): Promise<void> => { 59 const data = new Uint8Array(new TextEncoder().encode(overlayContent)); 60 let uploadedLength = 0; 61 62 const sendChunk = async (): Promise<unknown> => { changed this line in version 2 of the diff
1 /* eslint-disable no-magic-numbers */ changed this line in version 2 of the diff
19 ), 20 { 21 store, 22 } 23 ); 24 }; 25 26 describe('UserOverlayForm', () => { 27 it('renders the UserOverlayForm component', () => { 28 renderComponent(); 29 30 expect(screen.getByTestId('overlay-name')).toBeInTheDocument(); 31 expect(screen.getByLabelText('upload overlay')).toBeInTheDocument(); 32 }); 33 34 it('submits the form when Upload button is clicked', async () => { changed this line in version 2 of the diff
24 const [name, setName] = useState(''); 25 const [type, setType] = useState<SelectorItem>(DEFAULT_TYPE); 26 const [group, setGroup] = useState<SelectorItem>(DEFAULT_GROUP); 27 const [description, setDescription] = useState(''); 28 const [uploadedFile, setUploadedFile] = useState<File | null>(null); 29 const [elementsList, setElementsList] = useState(''); 30 const [overlayContent, setOverlayContent] = useState(''); 31 const projectId = useAppSelector(projectIdSelector); 32 const loadingAddOverlayStatus = useAppSelector(loadingAddOverlay); 33 const isPending = loadingAddOverlayStatus === 'pending'; 34 35 const handleSubmit = async (): Promise<void> => { 36 let filename = uploadedFile?.name; 37 38 if (!filename) { 39 filename = 'unknown.txt'; changed this line in version 2 of the diff
58 setOverlayContent(''); 59 setUploadedFile(null); 60 }; 61 62 const navigateToOverlays = (): void => { 63 dispatch(openOverlaysDrawer()); 64 }; 65 66 const handleChangeName = (e: ChangeEvent<HTMLInputElement>): void => { 67 setName(e.target.value); 68 }; 69 const handleChangeDescription = (e: ChangeEvent<HTMLTextAreaElement>): void => { 70 setDescription(e.target.value); 71 }; 72 73 const handleOverlayChange = (nameType: string, value: string): void => { changed this line in version 2 of the diff
79 const foundType = OVERLAY_TYPES.find(el => el.id === value); 80 if (foundType) { 81 setType(foundType); 82 } 83 } 84 }; 85 86 const handleChangeType = (value: SelectorItem): void => { 87 setType(value); 88 }; 89 90 const handleChangeGroup = (value: SelectorItem): void => { 91 setGroup(value); 92 }; 93 94 const handleChangeElementsList = (e: ChangeEvent<HTMLTextAreaElement>): void => { -
I don't understand what's happening inside the function
- parseOverlay returns void, and has side effect of setting state based on callback. Just by reading the code I have no idea why you do it. I would either change naming to be more descreptive/change implementation to be more understandable/add comments that describe your intention if two previous steps are not possible. Another idea of improving it might be using useReduce to handle parsing just before state update 2.What is difference between overlayContent and elementsList if it both sets the same values?
changed this line in version 2 of the diff
Renamed, added comments, some tests. Difference between overlayContent and elementsList is: We send the last content overlay added by the user. Having an OverlayContent allows us to keep track of what content was added last, because a user can upload a file, but then add elements list and the list should be sent as the content of the file, not the actual content of the file that the user uploaded earlier. And of course it works the other way around - if the user adds elements list and then uploads the file, the content of the file will be uploaded to the backend, not the elements list.
13 14 dispatch(openLoginModal()); 14 15 }; 15 16 17 const handleAddOverlay = (): void => { 18 dispatch(displayAddOverlaysDrawer()); 19 }; 20 16 21 return ( 17 22 <div className="p-6"> 18 23 {loadingUser === 'pending' && <h1>Loading</h1>} 19 24 20 25 {loadingUser !== 'pending' && !authenticatedUser && ( changed this line in version 2 of the diff
54 type UploadContentArgs = { 55 createdFile: CreatedOverlayFile; 56 overlayContent: string; 57 }; 58 const uploadContent = async ({ createdFile, overlayContent }: UploadContentArgs): Promise<void> => { 59 const data = new Uint8Array(new TextEncoder().encode(overlayContent)); 60 let uploadedLength = 0; 61 62 const sendChunk = async (): Promise<unknown> => { 63 if (uploadedLength >= data.length) { 64 return; 65 } 66 67 const chunk = data.slice(uploadedLength, uploadedLength + CHUNK_SIZE); 68 69 const responeJSON = await fetch( Yes, I tried to use axios and without a specific Content-Type it didn't work, I tried to set a specific Content-Type, unfortunately I can't find the correct one (I think I used almost all of them). This is probably related to the fact that we send it to the backend in the form of chunks, while fetch handles it without the Content-Type setting for the 1st time. I tried to check Content-Type in the network tab but it is not explicitly provided.
Unless I missed something :/
