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.

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

HMS-5036: Update upload modal to allow cancellation #402

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

Andrewgdewar
Copy link
Member

@Andrewgdewar Andrewgdewar commented Dec 19, 2024

Summary

  • Adds ability to cancel and restart downloads
  • Updates left side file icon to match color
  • Left side icon now has a tooltip to explain how this file is being uploaded/if the file data has been reused (pending copy feedback)
  • Added an overall upload progress bar.
  • Removed the extra spinner to the left side of the toggle dropdown menu (redundant).

Screenshot 2024-12-19 at 1 40 50 PM
Screenshot 2024-12-19 at 1 10 07 PM
Screenshot 2024-12-19 at 1 00 45 PM

Testing steps

Once the above are merged (or checked the backend pr 5126 is checked out)

  • Create an upload repository

  • Upload a larger file (here is a good example list)

  • While it is uploading, hit the cancel button. (can use network tab to turn on slow3g if necessary)

  • You can now either close the file, or retry the download (to have it pull the completed chunk list from the backend and restart)

  • Any combination of resuming/stopping/closing the modal/opening the modal/ reuploading SHOULD NOT result in any API error response. If one is seen during testing at any point, let me know.

@Andrewgdewar Andrewgdewar changed the title Hms 5036 Fixes 5036: Update upload modal to allow cancellation Dec 19, 2024
@Andrewgdewar Andrewgdewar changed the title Fixes 5036: Update upload modal to allow cancellation Fixes 5036: Update upload modal to allow cancellation Dec 19, 2024
@Andrewgdewar Andrewgdewar changed the title Fixes 5036: Update upload modal to allow cancellation HMS-5036: Update upload modal to allow cancellation Dec 19, 2024
@jlsherrill
Copy link
Member

@swadeley
Copy link
Member

swadeley commented Jan 8, 2025

@Andrewgdewar Please rebase

Copy link
Contributor

@xbhouse xbhouse left a comment

Choose a reason for hiding this comment

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

another small thing - i see the icons are clickable but show the tooltip on hover. should they be clickable?

Copy link
Contributor

@xbhouse xbhouse left a comment

Choose a reason for hiding this comment

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

couple small nitpicks and needs a rebase, but overall this looks and works great!! nice job 🎉

@Andrewgdewar
Copy link
Member Author

another small thing - i see the icons are clickable but show the tooltip on hover. should they be clickable?

This is likely from the patternfly component itself, and is likely due to accessibility requirements across platform (mobile) where there are no hover events.

Hover is not a thing on touch devices, so anyone on a phone, tablet, or touch-notebook will be better off with the touch action causing the tooltip to appear.

One thing I did change, was adding tabIndex={-1} to the file icons themselves, as I don't believe they need to be targetable via tabbing for screen readers, generally you want to only give tabbable elements actions, and the icons don't contain enough information for the screen reader to translate what we need here.

@xbhouse
Copy link
Contributor

xbhouse commented Jan 10, 2025

another small thing - i see the icons are clickable but show the tooltip on hover. should they be clickable?

This is likely from the patternfly component itself, and is likely due to accessibility requirements across platform (mobile) where there are no hover events.

Hover is not a thing on touch devices, so anyone on a phone, tablet, or touch-notebook will be better off with the touch action causing the tooltip to appear.

One thing I did change, was adding tabIndex={-1} to the file icons themselves, as I don't believe they need to be targetable via tabbing for screen readers, generally you want to only give tabbable elements actions, and the icons don't contain enough information for the screen reader to translate what we need here.

ahh right, thanks for pointing that out! makes sense 👍

@mayurilahane
Copy link
Contributor

/retest

@mayurilahane
Copy link
Contributor

mayurilahane commented Jan 13, 2025

  • Adds ability to cancel and restart downloads

Screenshot from 2025-01-13 15-38-43

@mayurilahane
Copy link
Contributor

mayurilahane commented Jan 13, 2025

  • Updates left side file icon to match color
  • [ ]
    image

@mayurilahane mayurilahane merged commit a047405 into content-services:main Jan 13, 2025
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants