Switched order of attribute initialization and DOM addition.

Fixes #6218.
This commit is contained in:
Jan-Willem Spuij 2020-07-16 15:10:15 +02:00 committed by Steve Sanderson
parent ec4b2221c2
commit 4b943417a7
11 changed files with 89 additions and 15 deletions

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@ -238,7 +238,8 @@ export class BrowserRenderer {
document.createElementNS('http://www.w3.org/2000/svg', tagName) : document.createElementNS('http://www.w3.org/2000/svg', tagName) :
document.createElement(tagName); document.createElement(tagName);
const newElement = toLogicalElement(newDomElementRaw); const newElement = toLogicalElement(newDomElementRaw);
insertLogicalChild(newDomElementRaw, parent, childIndex);
let inserted = false;
// Apply attributes // Apply attributes
const descendantsEndIndexExcl = frameIndex + frameReader.subtreeLength(frame); const descendantsEndIndexExcl = frameIndex + frameReader.subtreeLength(frame);
@ -247,6 +248,8 @@ export class BrowserRenderer {
if (frameReader.frameType(descendantFrame) === FrameType.attribute) { if (frameReader.frameType(descendantFrame) === FrameType.attribute) {
this.applyAttribute(batch, componentId, newDomElementRaw, descendantFrame); this.applyAttribute(batch, componentId, newDomElementRaw, descendantFrame);
} else { } else {
insertLogicalChild(newDomElementRaw, parent, childIndex);
inserted = true;
// As soon as we see a non-attribute child, all the subsequent child frames are // As soon as we see a non-attribute child, all the subsequent child frames are
// not attributes, so bail out and insert the remnants recursively // not attributes, so bail out and insert the remnants recursively
this.insertFrameRange(batch, componentId, newElement, 0, frames, descendantIndex, descendantsEndIndexExcl); this.insertFrameRange(batch, componentId, newElement, 0, frames, descendantIndex, descendantsEndIndexExcl);
@ -254,17 +257,40 @@ export class BrowserRenderer {
} }
} }
// We handle setting 'value' on a <select> in two different ways: // this element did not have any children, so it's not inserted yet.
// [1] When inserting a corresponding <option>, in case you're dynamically adding options if (!inserted) {
insertLogicalChild(newDomElementRaw, parent, childIndex);
}
// We handle setting 'value' on a <select> in three different ways:
// [1] When inserting a corresponding <option>, in case you're dynamically adding options.
// This is the case below.
// [2] After we finish inserting the <select>, in case the descendant options are being // [2] After we finish inserting the <select>, in case the descendant options are being
// added as an opaque markup block rather than individually // added as an opaque markup block rather than individually. This is the other case below.
// Right here we implement [2] // [3] In case the the value of the select and the option value is changed in the same batch.
if (newDomElementRaw instanceof HTMLSelectElement && selectValuePropname in newDomElementRaw) { // We just receive an attribute frame and have to set the select value afterwards.
if (newDomElementRaw instanceof HTMLOptionElement)
{
// Situation 1
this.trySetSelectValueFromOptionElement(newDomElementRaw);
} else if (newDomElementRaw instanceof HTMLSelectElement && selectValuePropname in newDomElementRaw) {
// Situation 2
const selectValue: string | null = newDomElementRaw[selectValuePropname]; const selectValue: string | null = newDomElementRaw[selectValuePropname];
setSelectElementValue(newDomElementRaw, selectValue); setSelectElementValue(newDomElementRaw, selectValue);
} }
} }
private trySetSelectValueFromOptionElement(optionElement: HTMLOptionElement) {
const selectElem = this.findClosestAncestorSelectElement(optionElement);
if (selectElem && (selectValuePropname in selectElem) && selectElem[selectValuePropname] === optionElement.value) {
setSelectElementValue(selectElem, optionElement.value);
delete selectElem[selectValuePropname];
return true;
}
return false;
}
private insertComponent(batch: RenderBatch, parent: LogicalElement, childIndex: number, frame: RenderTreeFrame) { private insertComponent(batch: RenderBatch, parent: LogicalElement, childIndex: number, frame: RenderTreeFrame) {
const containerElement = createAndInsertLogicalContainer(parent, childIndex); const containerElement = createAndInsertLogicalContainer(parent, childIndex);
@ -385,13 +411,10 @@ export class BrowserRenderer {
} else { } else {
element.removeAttribute('value'); element.removeAttribute('value');
} }
// See above for why we have this special handling for <select>/<option> // See above for why we have this special handling for <select>/<option>
// Note that this is only one of the two cases where we set the value on a <select> // Situation 3
const selectElem = this.findClosestAncestorSelectElement(element); this.trySetSelectValueFromOptionElement(<HTMLOptionElement>element);
if (selectElem && (selectValuePropname in selectElem) && selectElem[selectValuePropname] === value) {
this.tryApplyValueProperty(batch, selectElem, attributeFrame);
delete selectElem[selectValuePropname];
}
return true; return true;
} }
default: default:

View File

@ -216,6 +216,10 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests
Browser.Equal("Fourth", () => boundValue.Text); Browser.Equal("Fourth", () => boundValue.Text);
Assert.Equal("Fourth choice", target.SelectedOption.Text); Assert.Equal("Fourth choice", target.SelectedOption.Text);
// verify that changing an option value and selected value at the same time works.
Browser.FindElement(By.Id("change-variable-value")).Click();
Browser.Equal("Sixth", () => boundValue.Text);
// Verify we can select options whose value is empty // Verify we can select options whose value is empty
// https://github.com/dotnet/aspnetcore/issues/17735 // https://github.com/dotnet/aspnetcore/issues/17735
target.SelectByText("Empty value"); target.SelectByText("Empty value");

View File

@ -288,6 +288,17 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests
Browser.Contains(expectedMessage, () => errorLog.Text); Browser.Contains(expectedMessage, () => errorLog.Text);
} }
[Fact]
public void RenderAttributesBeforeConnectedCallBack()
{
Browser.MountTestComponent<RenderAttributesBeforeConnectedCallback>();
var element = Browser.FindElement(By.TagName("custom-web-component-data-from-attribute"));
var expectedContent = "success";
Browser.Contains(expectedContent, () => element.Text);
}
void SendKeysSequentially(IWebElement target, string text) void SendKeysSequentially(IWebElement target, string text)
{ {
// Calling it for each character works around some chars being skipped // Calling it for each character works around some chars being skipped

View File

@ -242,6 +242,7 @@
<optgroup label="Some choices"> <optgroup label="Some choices">
<!-- Show it also works with optgroup --> <!-- Show it also works with optgroup -->
<option value=@string.Empty>Empty value</option> <option value=@string.Empty>Empty value</option>
<option value="@variableValue">Variable value</option>
<option value=@SelectableValue.First>First choice</option> <option value=@SelectableValue.First>First choice</option>
<option value=@SelectableValue.Second>Second choice</option> <option value=@SelectableValue.Second>Second choice</option>
<option value=@SelectableValue.Third>Third choice</option> <option value=@SelectableValue.Third>Third choice</option>
@ -253,6 +254,7 @@
</select> </select>
<span id="select-box-value">@selectValue</span> <span id="select-box-value">@selectValue</span>
<button id="select-box-add-option" @onclick="AddAndSelectNewSelectOption">Add and select new item</button> <button id="select-box-add-option" @onclick="AddAndSelectNewSelectOption">Add and select new item</button>
<button id="change-variable-value" @onclick="ChangeVariableValueToSixth">Change variable value to Sixth.</button>
</p> </p>
<h2>Select (markup block)</h2> <h2>Select (markup block)</h2>
@ -383,13 +385,21 @@
DateTime? timeStepTextboxNullableDateTimeValue = null; DateTime? timeStepTextboxNullableDateTimeValue = null;
bool includeFourthOption = false; bool includeFourthOption = false;
enum SelectableValue { First, Second, Third, Fourth } enum SelectableValue { First, Second, Third, Fourth, Fifth, Sixth }
SelectableValue? selectValue = SelectableValue.Second; SelectableValue? selectValue = SelectableValue.Second;
SelectableValue? selectMarkupValue = SelectableValue.Second; SelectableValue? selectMarkupValue = SelectableValue.Second;
SelectableValue variableValue = SelectableValue.Fifth;
void AddAndSelectNewSelectOption() void AddAndSelectNewSelectOption()
{ {
includeFourthOption = true; includeFourthOption = true;
selectValue = SelectableValue.Fourth; selectValue = SelectableValue.Fourth;
} }
void ChangeVariableValueToSixth()
{
selectValue = SelectableValue.Sixth;
variableValue = SelectableValue.Sixth;
}
} }

View File

@ -70,6 +70,7 @@
<option value="BasicTestApp.RedTextComponent">Red text</option> <option value="BasicTestApp.RedTextComponent">Red text</option>
<option value="BasicTestApp.ReliabilityComponent">Server reliability component</option> <option value="BasicTestApp.ReliabilityComponent">Server reliability component</option>
<option value="BasicTestApp.RenderFragmentToggler">Render fragment renderer</option> <option value="BasicTestApp.RenderFragmentToggler">Render fragment renderer</option>
<option value="BasicTestApp.RenderAttributesBeforeConnectedCallback">Render attributes before ConnectedCallback</option>
<option value="BasicTestApp.ReorderingFocusComponent">Reordering focus retention</option> <option value="BasicTestApp.ReorderingFocusComponent">Reordering focus retention</option>
<option value="BasicTestApp.RouterTest.NavigationManagerComponent">NavigationManager Test</option> <option value="BasicTestApp.RouterTest.NavigationManagerComponent">NavigationManager Test</option>
<option value="BasicTestApp.RouterTest.TestRouter">Router</option> <option value="BasicTestApp.RouterTest.TestRouter">Router</option>

View File

@ -0,0 +1,14 @@
<h1>Render Attributes Before connectedCallback</h1>
<p>
When the connectedcallback event fires, it's nice to have the attributes of the HTML element set already,
as data is usually passed to the component using custom attributes.
</p>
<custom-web-component-data-from-attribute myattribute="@attributeString"></custom-web-component-data-from-attribute>
@code {
string attributeString = "success";
}

View File

@ -23,6 +23,7 @@
<!-- Used for specific test cases --> <!-- Used for specific test cases -->
<script src="js/jsinteroptests.js"></script> <script src="js/jsinteroptests.js"></script>
<script src="js/renderattributestest.js"></script>
<script src="js/webComponentPerformingJsInterop.js"></script> <script src="js/webComponentPerformingJsInterop.js"></script>
<script> <script>

View File

@ -0,0 +1,9 @@
// This web component is used from the RenderAttributesBeforeConnectedCallback test case
window.customElements.define('custom-web-component-data-from-attribute', class extends HTMLElement {
connectedCallback() {
let myattribute = this.getAttribute('myattribute') || 'failed';
this.innerHTML = myattribute;
}
});

View File

@ -16,6 +16,7 @@
<!-- Used for specific test cases --> <!-- Used for specific test cases -->
<script src="js/jsinteroptests.js"></script> <script src="js/jsinteroptests.js"></script>
<script src="js/renderattributestest.js"></script>
<script src="js/webComponentPerformingJsInterop.js"></script> <script src="js/webComponentPerformingJsInterop.js"></script>
<div id="blazor-error-ui"> <div id="blazor-error-ui">