Skip to content

Fix time/datetime-local binding with seconds when there's no step attribute #41868

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

Merged
merged 3 commits into from
May 27, 2022

Conversation

SteveSandersonMS
Copy link
Member

@SteveSandersonMS SteveSandersonMS commented May 26, 2022

Fixes #41731. This does appear to have been a regression in 6.0.

In 5.0, there was explicit JS-side logic to handle the case where an <input type=time> had no step attribute (which browsers interpret as "use whole number of minutes - no seconds"). We automatically stripped off any "seconds" part of the formatted string to make it value as far as the <input type=time> is concerned. This behavior had been there from the very first days of @bind. The implementation for this 5.0 behavior is here.

During 6.0, we lost this specific behavior while fixing an issue related to keyboard inputs, because unfortunately it looks like we hadn't created a good E2E test case originally to show that the seconds-stripping behavior was needed. This PR restores the 5.0-and-earlier handling for cases where there's no step attribute.

Risk: someone might be expecting to preserve the "seconds" value even in the absence of step. If a user doesn't try to edit the bound value, then the seconds would actually be preserved. However in that case the browser would already be showing a message saying the value is invalid, so it's unlikely the developer would be tolerating this, and it would be a bad UX even if they did.

@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner May 26, 2022 11:37
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label May 26, 2022
@SteveSandersonMS
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/2390621246

@github-actions
Copy link
Contributor

@SteveSandersonMS backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Add failing E2E test cases
Applying: Fix, plus further test case for datetime-local
Applying: Update JS files
warning: Cannot merge binary files: src/Components/Web.JS/dist/Release/blazor.webview.js (HEAD vs. Update JS files)
warning: Cannot merge binary files: src/Components/Web.JS/dist/Release/blazor.server.js (HEAD vs. Update JS files)
Using index info to reconstruct a base tree...
M	src/Components/Web.JS/dist/Release/blazor.server.js
M	src/Components/Web.JS/dist/Release/blazor.webview.js
Falling back to patching base and 3-way merge...
Auto-merging src/Components/Web.JS/dist/Release/blazor.webview.js
CONFLICT (content): Merge conflict in src/Components/Web.JS/dist/Release/blazor.webview.js
Auto-merging src/Components/Web.JS/dist/Release/blazor.server.js
CONFLICT (content): Merge conflict in src/Components/Web.JS/dist/Release/blazor.server.js
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 Update JS files
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@SteveSandersonMS SteveSandersonMS merged commit 8c5a59a into main May 27, 2022
@SteveSandersonMS SteveSandersonMS deleted the stevesa/input-date-no-step-fix branch May 27, 2022 09:12
@ghost ghost added this to the 7.0-preview6 milestone May 27, 2022
@SteveSandersonMS SteveSandersonMS added Done This issue has been fixed and removed Status:InProgress labels May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue when binding DateTime to input type="time"
2 participants