For checkboxes, bind to 'checked'. Fix special property handling in BrowserRenderer.ts. Fixes #659 and #703

* Stop the value<-->checked conversions for checkboxes. Just use native 'checked' property. Fixes #703

* Properly handle removal of 'checked' and 'value' attributes

* E2E coverage for removing 'value'
This commit is contained in:
Steve Sanderson 2018-05-01 16:40:07 +01:00 committed by GitHub
parent 644bd37e8c
commit f61ed4df4f
6 changed files with 98 additions and 37 deletions

View File

@ -84,7 +84,11 @@ export class BrowserRenderer {
const element = getLogicalChild(parent, childIndexAtCurrentDepth + siblingIndex);
if (element instanceof HTMLElement) {
const attributeName = renderTreeEdit.removedAttributeName(edit)!;
element.removeAttribute(attributeName);
// First try to remove any special property we use for this attribute
if (!this.tryApplySpecialProperty(element, attributeName, null)) {
// If that's not applicable, it's a regular DOM attribute so remove that
element.removeAttribute(attributeName);
}
} else {
throw new Error(`Cannot remove attribute from non-element child`);
}
@ -205,53 +209,76 @@ export class BrowserRenderer {
return;
}
if (attributeName === 'value') {
if (this.tryApplyValueProperty(toDomElement, renderTreeFrame.attributeValue(attributeFrame))) {
return; // If this DOM element type has special 'value' handling, don't also write it as an attribute
}
// First see if we have special handling for this attribute
if (!this.tryApplySpecialProperty(toDomElement, attributeName, attributeFrame)) {
// If not, treat it as a regular string-valued attribute
toDomElement.setAttribute(
attributeName,
renderTreeFrame.attributeValue(attributeFrame)!
);
}
// Treat as a regular string-valued attribute
toDomElement.setAttribute(
attributeName,
renderTreeFrame.attributeValue(attributeFrame)!
);
}
private tryApplyValueProperty(element: Element, value: string | null) {
private tryApplySpecialProperty(element: Element, attributeName: string, attributeFrame: RenderTreeFramePointer | null) {
switch (attributeName) {
case 'value':
return this.tryApplyValueProperty(element, attributeFrame);
case 'checked':
return this.tryApplyCheckedProperty(element, attributeFrame);
default:
return false;
}
}
private tryApplyValueProperty(element: Element, attributeFrame: RenderTreeFramePointer | null) {
// Certain elements have built-in behaviour for their 'value' property
switch (element.tagName) {
case 'INPUT':
case 'SELECT':
case 'TEXTAREA':
if (isCheckbox(element)) {
(element as HTMLInputElement).checked = value === '';
} else {
(element as any).value = value;
case 'TEXTAREA': {
const value = attributeFrame ? renderTreeFrame.attributeValue(attributeFrame) : null;
(element as any).value = value;
if (element.tagName === 'SELECT') {
// <select> is special, in that anything we write to .value will be lost if there
// isn't yet a matching <option>. To maintain the expected behavior no matter the
// element insertion/update order, preserve the desired value separately so
// we can recover it when inserting any matching <option>.
element[selectValuePropname] = value;
}
if (element.tagName === 'SELECT') {
// <select> is special, in that anything we write to .value will be lost if there
// isn't yet a matching <option>. To maintain the expected behavior no matter the
// element insertion/update order, preserve the desired value separately so
// we can recover it when inserting any matching <option>.
element[selectValuePropname] = value;
}
return true;
case 'OPTION':
element.setAttribute('value', value!);
}
case 'OPTION': {
const value = attributeFrame ? renderTreeFrame.attributeValue(attributeFrame) : null;
if (value) {
element.setAttribute('value', value);
} else {
element.removeAttribute('value');
}
// See above for why we have this special handling for <select>/<option>
const parentElement = element.parentElement;
if (parentElement && (selectValuePropname in parentElement) && parentElement[selectValuePropname] === value) {
this.tryApplyValueProperty(parentElement, value);
this.tryApplyValueProperty(parentElement, attributeFrame);
delete parentElement[selectValuePropname];
}
return true;
}
default:
return false;
}
}
private tryApplyCheckedProperty(element: Element, attributeFrame: RenderTreeFramePointer | null) {
// Certain elements have built-in behaviour for their 'checked' property
if (element.tagName === 'INPUT') {
const value = attributeFrame ? renderTreeFrame.attributeValue(attributeFrame) : null;
(element as any).checked = value !== null;
return true;
} else {
return false;
}
}
private insertFrameRange(componentId: number, parent: LogicalElement, childIndex: number, frames: System_Array<RenderTreeFramePointer>, startIndex: number, endIndexExcl: number): number {
const origChildIndex = childIndex;
for (let index = startIndex; index < endIndexExcl; index++) {
@ -281,10 +308,6 @@ function countDescendantFrames(frame: RenderTreeFramePointer): number {
}
}
function isCheckbox(element: Element) {
return element.tagName === 'INPUT' && element.getAttribute('type') === 'checkbox';
}
function raiseEvent(event: Event, browserRendererId: number, componentId: number, eventHandlerId: number, eventArgs: EventForDotNet<UIEventArgs>) {
event.preventDefault();

View File

@ -15,8 +15,7 @@ namespace Microsoft.AspNetCore.Blazor.Components
// when a specific type attribute is applied.
[BindInputElement(null, null, "value", "onchange")]
// For right now, the BrowserRenderer translates the value attribute to the checked attribute.
[BindInputElement("checkbox", null, "value", "onchange")]
[BindInputElement("checkbox", null, "checked", "onchange")]
[BindInputElement("text", null, "value", "onchange")]
[BindElement("select", null, "value", "onchange")]

View File

@ -15,7 +15,7 @@ namespace Test
base.BuildRenderTree(builder);
builder.OpenElement(0, "input");
builder.AddAttribute(1, "type", "checkbox");
builder.AddAttribute(2, "value", Microsoft.AspNetCore.Blazor.Components.BindMethods.GetValue(Enabled));
builder.AddAttribute(2, "checked", Microsoft.AspNetCore.Blazor.Components.BindMethods.GetValue(Enabled));
builder.AddAttribute(3, "onchange", Microsoft.AspNetCore.Blazor.Components.BindMethods.SetValueHandler(__value => Enabled = __value, Enabled));
builder.CloseElement();
}

View File

@ -2,7 +2,7 @@ Source Location: (86:2,12 [41] x:\dir\subdir\Test\TestComponent.cshtml)
|
public bool Enabled { get; set; }
|
Generated Location: (1037:23,12 [41] )
Generated Location: (1039:23,12 [41] )
|
public bool Enabled { get; set; }
|

View File

@ -28,14 +28,25 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
{
var target = Browser.FindElement(By.Id("textbox-initially-blank"));
var boundValue = Browser.FindElement(By.Id("textbox-initially-blank-value"));
var mirrorValue = Browser.FindElement(By.Id("textbox-initially-blank-mirror"));
var setNullButton = Browser.FindElement(By.Id("textbox-initially-blank-setnull"));
Assert.Equal(string.Empty, target.GetAttribute("value"));
Assert.Equal(string.Empty, boundValue.Text);
Assert.Equal(string.Empty, mirrorValue.GetAttribute("value"));
// Modify target; verify value is updated
// Modify target; verify value is updated and that textboxes linked to the same data are updated
target.SendKeys("Changed value");
Assert.Equal(string.Empty, boundValue.Text); // Doesn't update until change event
Assert.Equal(string.Empty, mirrorValue.GetAttribute("value"));
target.SendKeys("\t");
Assert.Equal("Changed value", boundValue.Text);
Assert.Equal("Changed value", mirrorValue.GetAttribute("value"));
// Remove the value altogether
setNullButton.Click();
Assert.Equal(string.Empty, target.GetAttribute("value"));
Assert.Equal(string.Empty, boundValue.Text);
Assert.Equal(string.Empty, mirrorValue.GetAttribute("value"));
}
[Fact]
@ -43,13 +54,23 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
{
var target = Browser.FindElement(By.Id("textbox-initially-populated"));
var boundValue = Browser.FindElement(By.Id("textbox-initially-populated-value"));
var mirrorValue = Browser.FindElement(By.Id("textbox-initially-populated-mirror"));
var setNullButton = Browser.FindElement(By.Id("textbox-initially-populated-setnull"));
Assert.Equal("Hello", target.GetAttribute("value"));
Assert.Equal("Hello", boundValue.Text);
Assert.Equal("Hello", mirrorValue.GetAttribute("value"));
// Modify target; verify value is updated
// Modify target; verify value is updated and that textboxes linked to the same data are updated
target.Clear();
target.SendKeys("Changed value\t");
Assert.Equal("Changed value", boundValue.Text);
Assert.Equal("Changed value", mirrorValue.GetAttribute("value"));
// Remove the value altogether
setNullButton.Click();
Assert.Equal(string.Empty, target.GetAttribute("value"));
Assert.Equal(string.Empty, boundValue.Text);
Assert.Equal(string.Empty, mirrorValue.GetAttribute("value"));
}
[Fact]
@ -86,6 +107,7 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
{
var target = Browser.FindElement(By.Id("checkbox-initially-unchecked"));
var boundValue = Browser.FindElement(By.Id("checkbox-initially-unchecked-value"));
var invertButton = Browser.FindElement(By.Id("checkbox-initially-unchecked-invert"));
Assert.False(target.Selected);
Assert.Equal("False", boundValue.Text);
@ -93,6 +115,11 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
target.Click();
Assert.True(target.Selected);
Assert.Equal("True", boundValue.Text);
// Modify data; verify checkbox is updated
invertButton.Click();
Assert.False(target.Selected);
Assert.Equal("False", boundValue.Text);
}
[Fact]
@ -100,6 +127,7 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
{
var target = Browser.FindElement(By.Id("checkbox-initially-checked"));
var boundValue = Browser.FindElement(By.Id("checkbox-initially-checked-value"));
var invertButton = Browser.FindElement(By.Id("checkbox-initially-checked-invert"));
Assert.True(target.Selected);
Assert.Equal("True", boundValue.Text);
@ -107,6 +135,11 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
target.Click();
Assert.False(target.Selected);
Assert.Equal("False", boundValue.Text);
// Modify data; verify checkbox is updated
invertButton.Click();
Assert.True(target.Selected);
Assert.Equal("True", boundValue.Text);
}
[Fact]

View File

@ -4,11 +4,15 @@
Initially blank:
<input id="textbox-initially-blank" bind="textboxInitiallyBlankValue" />
<span id="textbox-initially-blank-value">@textboxInitiallyBlankValue</span>
<input id="textbox-initially-blank-mirror" bind="textboxInitiallyBlankValue" readonly />
<button id="textbox-initially-blank-setnull" onclick="@(() => { textboxInitiallyBlankValue = null; })">Set null</button>
</p>
<p>
Initially populated:
<input id="textbox-initially-populated" bind="textboxInitiallyPopulatedValue" />
<span id="textbox-initially-populated-value">@textboxInitiallyPopulatedValue</span>
<input id="textbox-initially-populated-mirror" bind="textboxInitiallyPopulatedValue" readonly />
<button id="textbox-initially-populated-setnull" onclick="@(() => { textboxInitiallyPopulatedValue = null; })">Set null</button>
</p>
<h2>Text Area</h2>
@ -28,11 +32,13 @@
Initially unchecked:
<input id="checkbox-initially-unchecked" bind="checkboxInitiallyUncheckedValue" type="checkbox" />
<span id="checkbox-initially-unchecked-value">@checkboxInitiallyUncheckedValue</span>
<button id="checkbox-initially-unchecked-invert" onclick="@(() => { checkboxInitiallyUncheckedValue = !checkboxInitiallyUncheckedValue; })">Invert</button>
</p>
<p>
Initially checked:
<input id="checkbox-initially-checked" bind="checkboxInitiallyCheckedValue" type="checkbox" />
<span id="checkbox-initially-checked-value">@checkboxInitiallyCheckedValue</span>
<button id="checkbox-initially-checked-invert" onclick="@(() => { checkboxInitiallyCheckedValue = !checkboxInitiallyCheckedValue; })">Invert</button>
</p>
<h2>Select</h2>