Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Sample File Upload and upgrade buttons to Shadcn button #9157

Merged
merged 29 commits into from
Dec 3, 2024
Merged

Update Sample File Upload and upgrade buttons to Shadcn button #9157

Changes from 3 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8448989
Added Clear Image functionality
nithish1018 Nov 19, 2024
662f437
removed unneccessary line
nithish1018 Nov 19, 2024
cef5438
Added bot suggestions
nithish1018 Nov 19, 2024
ec36ef5
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 20, 2024
a7043f6
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 21, 2024
a341da1
Fixed search icon overlay
nithish1018 Nov 21, 2024
5403593
removing unrelated change
nithish1018 Nov 21, 2024
9d3de71
Updated file upload with useFileUpload hook
nithish1018 Nov 22, 2024
bae2649
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 22, 2024
52100c4
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 25, 2024
18ad9b5
Added types for sample
nithish1018 Nov 26, 2024
180fa67
Merge branch 'develop' into issues/9153/addclearimage
shivankacker Nov 26, 2024
ceba9fb
\Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 27, 2024
c972c5b
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 27, 2024
5489cb6
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 28, 2024
3c6eb0e
Updated the file upload UI
nithish1018 Nov 28, 2024
b48a7e3
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 29, 2024
3801776
Updated Button to shadcn button
nithish1018 Nov 29, 2024
6e180b3
removed unnecessary line
nithish1018 Nov 29, 2024
4598c0c
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 29, 2024
28ccbc4
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 30, 2024
fd0bf27
removed remove icon in favour of discard button
nithish1018 Nov 30, 2024
e86f229
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 30, 2024
66cf623
updated primary button color
nithish1018 Nov 30, 2024
48a604f
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Nov 30, 2024
37cfc0e
fixed eslint issues
nithish1018 Nov 30, 2024
76ebf4f
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Dec 1, 2024
0af4cf0
Merge branch 'develop' into issues/9153/addclearimage
nithish1018 Dec 2, 2024
cd09c74
removed button array
nithish1018 Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/components/Patient/UpdateStatusDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const UpdateStatusDialog = (props: Props) => {
const [contentType, setcontentType] = useState<string>("");
const [uploadPercent, setUploadPercent] = useState(0);
const [uploadStarted, setUploadStarted] = useState<boolean>(false);
const [uploadDone, setUploadDone] = useState<boolean>(false);

const currentStatus = SAMPLE_TEST_STATUS.find(
(i) => i.text === sample.status,
Expand Down Expand Up @@ -123,7 +122,6 @@ const UpdateStatusDialog = (props: Props) => {
(xhr: XMLHttpRequest) => {
if (xhr.status >= 200 && xhr.status < 300) {
setUploadStarted(false);
setUploadDone(true);
request(routes.editUpload, {
pathParams: {
id: data.id,
Expand Down Expand Up @@ -156,13 +154,15 @@ const UpdateStatusDialog = (props: Props) => {
);
return e.target.files[0];
};
const removeFile = () => {
setfile(undefined);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

🛠️ Refactor suggestion

Add upload state check to prevent file removal during upload

The verification reveals that uploadStarted state is properly managed in the component and is used during file uploads. However, the removeFile function currently lacks this check, which could lead to inconsistent state if a file is removed during upload.

  • Add upload state check in removeFile function to prevent file removal during active uploads
  • Consider adding proper ARIA attributes and visual feedback for better accessibility

The suggested changes in the review comment are valid and necessary:

  1. Prevent file removal during upload with the upload state check
  2. Add proper visual and accessibility attributes to the clear button
🔗 Analysis chain

Verify clear file behavior during upload

Please ensure that the clear file functionality is properly disabled when a file upload is in progress to prevent potential issues.

Consider adding an upload state check:

 const removeFile = () => {
+  if (uploadStarted) return;
   setfile(undefined);
   setcontentType("");
 };
 {file && (
   <CareIcon
     icon="l-times"
     className="text-lg cursor-pointer mt-2 mr-4"
     onClick={removeFile}
+    style={{ opacity: uploadStarted ? 0.5 : 1 }}
+    role="button"
+    aria-disabled={uploadStarted}
   />
 )}

Also applies to: 239-246

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any upload state checks before allowing file removal
rg -A 5 "uploadStarted.*removeFile|removeFile.*uploadStarted" .

Length of output: 65

Script:

#!/bin/bash
# Let's first check if uploadStarted state exists in the file
rg -A 5 "uploadStarted|setUploadStarted" src/components/Patient/UpdateStatusDialog.tsx

# Also check the component's state management
ast-grep --pattern 'const [$_] = useState<$_>($$$)'  src/components/Patient/UpdateStatusDialog.tsx

Length of output: 1590

const handleUpload = async () => {
const f = file;
if (f === undefined) return;
const category = "UNSPECIFIED";
const name = f.name;
setUploadStarted(true);
setUploadDone(false);

const { data } = await request(routes.createUpload, {
body: {
Expand Down Expand Up @@ -236,11 +236,14 @@ const UpdateStatusDialog = (props: Props) => {
hidden
/>
</label>
<Submit
type="submit"
onClick={handleUpload}
disabled={uploadDone}
>
{file && (
<CareIcon
icon="l-times"
className="text-lg cursor-pointer mt-2 mr-4"
onClick={removeFile}
/>
)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance accessibility for the clear file button

The clear file icon button needs accessibility improvements to help screen reader users understand its purpose.

 {file && (
   <CareIcon
     icon="l-times"
     className="text-lg cursor-pointer mt-2 mr-4"
     onClick={removeFile}
+    role="button"
+    aria-label={t("clear_file")}
+    title={t("clear_file")}
   />
 )}

Committable suggestion skipped: line range outside the PR's diff.

<Submit type="submit" onClick={handleUpload} disabled={!file}>
<CareIcon icon="l-cloud-upload" className="text-lg" />
<span>Upload</span>
</Submit>
Expand Down
Loading