Updated Document Upload and details #3643
Conversation
|
✅ No secrets were detected in the code. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## dev #3643 +/- ##
==========================================
+ Coverage 69.47% 74.28% +4.81%
==========================================
Files 1372 902 -470
Lines 34356 19045 -15311
Branches 6474 5443 -1031
==========================================
- Hits 23868 14148 -9720
+ Misses 10232 4897 -5335
+ Partials 256 0 -256
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@FuriousLlama there are some minor variations compared to the mockup that I see in https://jira.th.gov.bc.ca/browse/PSP-7338 Can you please confirm that what you have in this story is approved by Ana, and if so, get here to update the mockups so we don't get test failures. |
| // handle validations | ||
| if (changeEvent.target !== null) { | ||
| var target = changeEvent.target; | ||
| if (target.files !== null) { |
There was a problem hiding this comment.
target.files?.length !== null & target.files.length >= 1
(or similar)
There was a problem hiding this comment.
sure, added the check for at least one file
| setIsDragging(false); | ||
| }; | ||
|
|
||
| const shortenString = (text: string, maxlenght: number, terminator: string = '...'): string => { |
There was a problem hiding this comment.
maxlenght -> maxLength
| {selectedFile !== null && ( | ||
| <StyledSelectedFile> | ||
| <SectionField label="File to add" labelWidth="3"> | ||
| {shortenString(selectedFile.name || '', 20)} |
There was a problem hiding this comment.
maybe could use the OverflowTip component instead?
There was a problem hiding this comment.
I checked with ana and this way worked better
| validExtensions: string[]; | ||
| } | ||
|
|
||
| const FileDragAndDrop: FunctionComponent<React.PropsWithChildren<IFileDragAndDropProps>> = ({ |
There was a problem hiding this comment.
The test coverage on this isn't great. Unit testing this completely may not be realistic but is there anything we can do here?
| )} | ||
| </Row> | ||
|
|
||
| <Section |
There was a problem hiding this comment.
This seems duplicated from the DocumentDetailHeader
There was a problem hiding this comment.
Not sure what you mean.
| })); | ||
|
|
||
| const onDocumentSelected = () => { | ||
| debugger; |
There was a problem hiding this comment.
thinking this was not intentional.
|
✅ No secrets were detected in the code. |
|
✅ No secrets were detected in the code. |
| @@ -215,12 +217,19 @@ export const GenericModal = (props: Omit<BsModalProps, 'onHide'> & ModalProps) = | |||
| title="cancel-modal" | |||
| variant={cancelButtonVariant ?? 'secondary'} | |||
| onClick={close} | |||
| test-id="cancel-modal-button" | |||
There was a problem hiding this comment.
we already have the title property to get the buttons in tests. not opposed to adding test ids but I believe the right syntax is data-testid="..."
See here: https://testing-library.com/docs/queries/bytestid/
| <Button | ||
| title="ok-modal" | ||
| variant={okButtonVariant ?? 'primary'} | ||
| onClick={ok} | ||
| disabled={handleOkDisabled} | ||
| test-id="ok-modal-button" | ||
| > |
There was a problem hiding this comment.
Same comment re: test-id
|
✅ No secrets were detected in the code. |
1 similar comment
|
✅ No secrets were detected in the code. |
|
|
|
|
|
@FuriousLlama just trying to save Praveen some time, if Ana and you have already talked and she approved your version of the modal, she needs to update her mockup or Praveen will log bugs stating the mockup doesn't match - lets get ahead of that. |
|
✅ No secrets were detected in the code. |
1 similar comment
|
✅ No secrets were detected in the code. |
|
✅ No secrets were detected in the code. |
No description provided.