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

Upload Progress Bar #1502

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Conversation

feriel97
Copy link
Contributor

@feriel97 feriel97 commented Feb 24, 2025

Motivation and Context

fixes issue #1213

Description

added a progress bar for the upload of videos in the admin panel

Steps for Testing

Prerequisites:

  • 1 Lecturer
  • 1 Lecture
  • 1 Video to upload
  1. Log in
  2. Navigate to the admin panel
  3. Go to the desired course
  4. Create a new lecture

Screenshots

upload-progress-2

@feriel97 feriel97 changed the title added progress bar Upload Progress Bar Feb 24, 2025
@feriel97 feriel97 marked this pull request as ready for review February 24, 2025 19:49
Copy link
Contributor

@karjo24 karjo24 left a comment

Choose a reason for hiding this comment

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

The rest looks good. However, I was not able to test this not locally, as this would require setting up the worker.

<li>Presentation Video: <b x-text="progress.PRES === null ? 'skipped' : `${progress.PRES}%`"></b></li>
<li>Camera Video: <b x-text="progress.CAM === null ? 'skipped' : `${progress.CAM}%`"></b></li>
<li>Combined Video:
<div x-data="{ currentVal: progress.COMB, minVal: 0, maxVal: 100 }" class="flex h-4 w-full overflow-hidden rounded-radius bg-surface-alt dark:bg-surface-dark-alt" role="progressbar" aria-label="default progress bar" :aria-valuenow="currentVal" :aria-valuemin="minVal" :aria-valuemax="maxVal">
Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation recommends to use the native element whenever possible. If this is possible and doesn't overcomplicate things, we should probably follow this advice.

If we keep the role, we can leave out the :aria-valuemin="minVal" :aria-valuemax="maxVal" as this defaults to 0 and 100 respectively.

</div>
</li>
<li>Presentation Video:
<div x-data="{ currentVal: progress.PRES, minVal: 0, maxVal: 100 }" class="flex h-4 w-full overflow-hidden rounded-radius bg-surface-alt dark:bg-surface-dark-alt" role="progressbar" aria-label="default progress bar" :aria-valuenow="currentVal" :aria-valuemin="minVal" :aria-valuemax="maxVal">
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

</div>
</li>
<li>Camera Video:
<div x-data="{ currentVal: progress.CAM, minVal: 0, maxVal: 100 }" class="flex h-4 w-full overflow-hidden rounded-radius bg-surface-alt dark:bg-surface-dark-alt" role="progressbar" aria-label="default progress bar" :aria-valuenow="currentVal" :aria-valuemin="minVal" :aria-valuemax="maxVal">
Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants