Skip to content

Fix unable to set progress value to 0#2757

Merged
marvinhagemeister merged 1 commit into
masterfrom
progress-value
Sep 20, 2020
Merged

Fix unable to set progress value to 0#2757
marvinhagemeister merged 1 commit into
masterfrom
progress-value

Conversation

@marvinhagemeister

Copy link
Copy Markdown
Member

Due to a weird oddity in the DOM interface the value 0 is only treated as 0 when the value attribute is present. When the value is not present the progress bar is treated as being indeterminate. But the problem is that the DOM interface will always have all properties and the initial value for value is 0. This mismatch caused us to never set the progress bar value to 0

Fixes #2756 .

@github-actions

github-actions Bot commented Sep 19, 2020

Copy link
Copy Markdown

Size Change: +64 B (0%)

Total Size: 40.8 kB

Filename Size Change
dist/preact.js 4.08 kB +15 B (0%)
dist/preact.min.js 4.11 kB +16 B (0%)
dist/preact.module.js 4.1 kB +16 B (0%)
dist/preact.umd.js 4.14 kB +17 B (0%)
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.17 kB 0 B
compat/dist/compat.module.js 3.18 kB 0 B
compat/dist/compat.umd.js 3.23 kB 0 B
debug/dist/debug.js 3.09 kB 0 B
debug/dist/debug.module.js 3.09 kB 0 B
debug/dist/debug.umd.js 3.18 kB 0 B
devtools/dist/devtools.js 185 B 0 B
devtools/dist/devtools.module.js 195 B 0 B
devtools/dist/devtools.umd.js 261 B 0 B
hooks/dist/hooks.js 1.1 kB 0 B
hooks/dist/hooks.module.js 1.12 kB 0 B
hooks/dist/hooks.umd.js 1.18 kB 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.module.js 439 B 0 B
test-utils/dist/testUtils.umd.js 515 B 0 B

compressed-size-action

@coveralls

coveralls commented Sep 19, 2020

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling 54c006d on progress-value into b17a932 on master.

Comment thread src/diff/index.js
// despite the attribute not being present. When the attribute
// is missing the progress bar is treated as indeterminate.
// To fix that we'll always update it when it is 0 for progress elements
(i !== dom.value || (newVNode.type === 'progress' && !i))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Turns out my previous fix of doing !dom.hasAttribute('value') is not a good one as it leads to all input values always being updated. The new fix only updates it, despite dom.value === props.value when the value is 0 for progress elements.

@marvinhagemeister marvinhagemeister merged commit 134f65c into master Sep 20, 2020
@marvinhagemeister marvinhagemeister deleted the progress-value branch September 20, 2020 08:12
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.

Setting the value of a progress element to 0 removes the attribute

3 participants