-
Notifications
You must be signed in to change notification settings - Fork 187
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
Fix default upload duration being shown in raw seconds #3470
Fix default upload duration being shown in raw seconds #3470
Conversation
@rtibbles the above mentioned issue on PR description is fixed. Btw, there's a bug that I observed while I was fixing this issue. On uploading a new file when there's a already uploaded file, the If we can make ActivityDuration react whenever a new file is uploaded we can fix it I think? |
Ah, that's a good catch - can you file a follow up issue for that case? |
@@ -167,6 +167,20 @@ | |||
handleInput(value) { | |||
this.$emit('input', this.convertToSeconds(value)); | |||
}, | |||
convertToHHMMSS(totalSeconds) { | |||
if (totalSeconds !== null && Number.isInteger(totalSeconds)) { | |||
const hours = Math.floor(totalSeconds / 3600) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method will give the same result and seems a little simpler: https://stackoverflow.com/a/25279399
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That implementation breaks when input seconds are greater than or equal to 86400 seconds.
On 86400 seconds, it outputs 00:00:00. The expected output should be 24:00:00. Date objects are relative to specific parameters like timezone, etc. And our use case is absolute in nature. So, IMO the current implementation feels more robust to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An excellent point!
An issue for the outdated completion criteria duration: #3472. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it does the trick!
Summary
This PR fixes the default upload duration being shown in raw seconds. Now, we show the
duration
inHH:MM:SS
format.Screenshots (if applicable)
Before:
![image](https://user-images.githubusercontent.com/26724128/181917512-b9cbb70d-444f-4c26-b8b9-d745131e6a88.png)
After:
![image](https://user-images.githubusercontent.com/26724128/181917571-78df39c5-c560-4761-a2f2-ee43a7dd4cb7.png)
Reviewer guidance
References
Closes #3465.
Contributor's Checklist
PR process:
CHANGELOG
label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocs
label has been added if this introduces a change that needs to be updated in the user docs?requirements.txt
files also included in this PRStudio-specifc:
notranslate
class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages
,components
, andlayouts
directories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarn
andpip
)