Add warmup and step delay to developer UI #323
Conversation
camillobruni
added
the
trivial change
|
Please have a look again. |
camillobruni
changed the title
|
Also updated the slider UI: |
resources/developer-mode.mjs
Outdated
| content.append(createUIForWarmupBeforeSync()) | ||
| content.append(document.createElement("br")); | ||
| content.append(createUIForSyncStepDelay()); | ||
| details.append(content); |
There was a problem hiding this comment.
details.append(content) is already done below, so probably this line should be removed
resources/developer-mode.mjs
Outdated
| range.value = initialValue; | ||
|
|
||
| const label = document.createElement("label"); | ||
| const rangeLabel = document.createElement("span"); |
There was a problem hiding this comment.
nit: the name "label" can be confusing in this context, I believe it should be renamed, for example to rangeValueText?
There was a problem hiding this comment.
I wonder if rangeLabel should be outside of the label element (it's not labeling the input), but this would need more CSS changes that we're willing to do here, I believe.
There was a problem hiding this comment.
Renamed the variable to rangeValueAndUnit, let's maybe move the rest for later.
| }; | ||
| return label; | ||
| } |
There was a problem hiding this comment.
I feel like we could factorize a bit more these UI creations, but I'm not sure this would actually be easier to read. So I'm fine with keeping them this way.
|
What's the status of this PR? Are we merging it for 3.0? |
|
I'd be happy to land this if possible, but only if everybody is fine with it :) |
|
Let's land it for 3.0 then :) |
Drive-by-fix: