OBPIH-6396 Full Outbound Import - Reusable file select field (frontend)#4646
OBPIH-6396 Full Outbound Import - Reusable file select field (frontend)#4646awalkowiak merged 3 commits intodevelopfrom
Conversation
awalkowiak
left a comment
awalkowiak
left a comment
There was a problem hiding this comment.
Overall I think it looks good to me, I am just a bit worried about the previous styling (for example looks like dropzone on the import location might lost its styling). If everything looks and works as before then I'm fine with it.
kchelstowski
left a comment
kchelstowski
left a comment
There was a problem hiding this comment.
My question would be if you checked the compatibility of the FileSelect you've implemented with both: react-hook-form and zod and if so, if there are any obstacles we need to know before trying to use one in the upcoming tickets?
|
|
||
| const validateFileType = (file) => { | ||
| if (!allowedExtensions.length || allowedExtensions.includes(getFileExtension(file))) { | ||
| return null; |
There was a problem hiding this comment.
I think for clarity of validation methods we used to have, this should eventually return true if the validation passes, which I think it does in this case if I understand it correctly:
"If we don't have any allowed extensions specified or the file extension is in the allowed ones, the validation should pass (currently returning null)"?
There was a problem hiding this comment.
The null is required by the react-dropzone. Basically, this validation blocks you from uploading files with the wrong extensions.
| onDrop, | ||
| noClick: true, | ||
| noKeyboard: true, | ||
| validator: validateFileType, |
There was a problem hiding this comment.
but I guess this hook from the library expects to return a falsy value if the validation passes, hence my previous comment doesn't make sense, since we are not able to adjust that?
There was a problem hiding this comment.
This is not the validation for our libraries, I answered it in the comment above. I tested it with our libraries and it is working fine. In case someone has some problems with adjusting it, it can also be done by the workaround by passing the setValue function.
| {data.path} | ||
| {' '} | ||
| ( | ||
| {data.size} | ||
| {' '} | ||
| bytes) | ||
| {file?.errors?.length ? ( | ||
| <ul> | ||
| {file.errors.map((e) => ( | ||
| <li key={e.code}>{e.message}</li> | ||
| ))} | ||
| </ul> | ||
| ) : null} |
There was a problem hiding this comment.
I would maybe do something like:
| {data.path} | |
| {' '} | |
| ( | |
| {data.size} | |
| {' '} | |
| bytes) | |
| {file?.errors?.length ? ( | |
| <ul> | |
| {file.errors.map((e) => ( | |
| <li key={e.code}>{e.message}</li> | |
| ))} | |
| </ul> | |
| ) : null} | |
| <div> | |
| <span className="mx-1" >{data.path}<span> | |
| <span className="mx-1" >({data.size})<span> | |
| </div> | |
| {!!file?.errors?.length ? ( | |
| <ul> | |
| {file.errors.map((e) => ( | |
| <li key={e.code}>{e.message}</li> | |
| ))} | |
| </ul> | |
| ) } |
But current solution is fine
There was a problem hiding this comment.
@drodzewicz I think you skipped the fallback (:), or you meant to do it via &&. Honestly I'd avoid double negation, since it is usually confusing.
I would prefer Alan's version.
4 participants
No description provided.