Skip to content

Conversation

@bsmth
Copy link
Member

@bsmth bsmth commented May 19, 2025

Description

This allows specifications to use ProgressEvents while not necessarily giving away the exact number of bytes, for example by using numbers between 0 and 1 … with 6 decimal digits

The docs are explicit about ints here, and changes have landed allowing doubles:

https://xhr.spec.whatwg.org/#dom-progresseventinit-loaded

Related issues and pull requests

Fixes mdn/mdn#641

@bsmth bsmth requested a review from a team as a code owner May 19, 2025 11:24
@bsmth bsmth requested review from wbamberg and removed request for a team May 19, 2025 11:24
@github-actions github-actions bot added Content:WebAPI Web API docs size/s [PR only] 6-50 LoC changed labels May 19, 2025
@bsmth
Copy link
Member Author

bsmth commented May 19, 2025

CC @domenic

@github-actions
Copy link
Contributor

github-actions bot commented May 19, 2025

Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

This would need BCD too I reckon, +1 for content, but not sure if it fixes the issue completely.

Also as a reader I would be curious to know what a fraction of a byte even means.

@bsmth bsmth changed the title chore: Change ints to doubles for ProgressEvent properties chore: Change ints to 'number' for ProgressEvent properties May 30, 2025
@bsmth
Copy link
Member Author

bsmth commented May 30, 2025

Also as a reader I would be curious to know what a fraction of a byte even means.

Yeah that's a good point, motivation is:

This allows specifications to use ProgressEvents while not necessarily giving away the exact number of bytes, for example by using numbers between 0 and 1 … with 6 decimal digits

https://webmachinelearning.github.io/writing-assistance-apis/#note-download-progress-fraction

@bsmth
Copy link
Member Author

bsmth commented Jun 2, 2025

@Josh-Cena do you want to have another look? I've made a deeper tidy at 9291ddb.

Some of the bytes-specific wording was added here:

Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

If I understand correctly, for ProgressEvents dispatched by the browser you still get sizes in bytes; for ProgressEvents created by yourself (which I'm not even sure happens a lot) you can plug in any number you want, not just 0–1 normalized. Perhaps you want to make that even clearer...? Leaving my +1

@bsmth
Copy link
Member Author

bsmth commented Jun 2, 2025

If I understand correctly, for ProgressEvents dispatched by the browser you still get sizes in bytes; for ProgressEvents created by yourself (which I'm not even sure happens a lot) you can plug in any number you want, not just 0–1 normalized. Perhaps you want to make that even clearer...? Leaving my +1

Yeah that's right, let me go over once again to make sure it's clear, tnx

@bsmth
Copy link
Member Author

bsmth commented Jun 2, 2025

I think this is good enough for a merge now to land the improvements. I may call this out for another set of eyes, too. Thanks, Josh 👍🏻

@bsmth bsmth merged commit 03ca44d into mdn:main Jun 2, 2025
8 checks passed
@bsmth bsmth deleted the mdn-641 branch June 2, 2025 16:21
estelle pushed a commit that referenced this pull request Jun 6, 2025
* chore: Change ints to doubles for ProgressEvent properties

* Apply suggestions from code review

* Apply suggestions from code review

* Update files/en-us/web/api/progressevent/loaded/index.md

* feat: Clarify when progressevents use decimals

* Apply suggestions from code review

Co-authored-by: Joshua Chen <[email protected]>

* feat: Clarify when progressevents use decimals

---------

Co-authored-by: Joshua Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed size/s [PR only] 6-50 LoC changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update ProgressEvent documentation to show it allows doubles

3 participants