From a02ac60c8c703c1d0a7a4b1799f52f86761ee1c6 Mon Sep 17 00:00:00 2001
From: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Date: Fri, 16 Mar 2018 09:37:47 +0000
Subject: [PATCH] Fix selection on <select> box. Fixes #157

---
 .../src/Rendering/BrowserRenderer.ts          | 19 +++++++++++++-
 .../Tests/BindTest.cs                         | 21 ++++++++++++++++
 .../BasicTestApp/BindCasesComponent.cshtml    | 25 +++++++++++++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts
index b4434ab092..1d16c5e489 100644
--- a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts
+++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts
@@ -2,6 +2,7 @@
 import { getRenderTreeEditPtr, renderTreeEdit, RenderTreeEditPointer, EditType } from './RenderTreeEdit';
 import { getTreeFramePtr, renderTreeFrame, FrameType, RenderTreeFramePointer } from './RenderTreeFrame';
 import { platform } from '../Environment';
+const selectValuePropname = '_blazorSelectValue';
 let raiseEventMethod: MethodHandle;
 let renderComponentMethod: MethodHandle;
 
@@ -226,8 +227,24 @@ export class BrowserRenderer {
         if (isCheckbox(element)) {
           (element as HTMLInputElement).checked = value === 'True';
         } else {
-          // Note: this doen't handle <select> correctly: https://github.com/aspnet/Blazor/issues/157
           (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;
+          }
+        }
+        return true;
+      case 'OPTION':
+        element.setAttribute('value', 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);
+          delete parentElement[selectValuePropname];
         }
         return true;
       default:
diff --git a/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/BindTest.cs b/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/BindTest.cs
index 99c1dde52e..8c20996df0 100644
--- a/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/BindTest.cs
+++ b/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/BindTest.cs
@@ -5,6 +5,7 @@ using BasicTestApp;
 using Microsoft.AspNetCore.Blazor.E2ETest.Infrastructure;
 using Microsoft.AspNetCore.Blazor.E2ETest.Infrastructure.ServerFixtures;
 using OpenQA.Selenium;
+using OpenQA.Selenium.Support.UI;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
@@ -74,5 +75,25 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
             Assert.False(target.Selected);
             Assert.Equal("False", boundValue.Text);
         }
+
+        [Fact]
+        public void CanBindSelect()
+        {
+            var target = new SelectElement(Browser.FindElement(By.Id("select-box")));
+            var boundValue = Browser.FindElement(By.Id("select-box-value"));
+            Assert.Equal("Second choice", target.SelectedOption.Text);
+            Assert.Equal("Second", boundValue.Text);
+
+            // Modify target; verify value is updated
+            target.SelectByText("Third choice");
+            Assert.Equal("Third", boundValue.Text);
+
+            // Also verify we can add and select new options atomically
+            // Don't move this into a separate test, because then the previous assertions
+            // would be dependent on test execution order (or would require a full page reload)
+            Browser.FindElement(By.Id("select-box-add-option")).Click();
+            Assert.Equal("Fourth", boundValue.Text);
+            Assert.Equal("Fourth choice", target.SelectedOption.Text);
+        }
     }
 }
diff --git a/test/testapps/BasicTestApp/BindCasesComponent.cshtml b/test/testapps/BasicTestApp/BindCasesComponent.cshtml
index c8527aa9e1..2a202900cb 100644
--- a/test/testapps/BasicTestApp/BindCasesComponent.cshtml
+++ b/test/testapps/BasicTestApp/BindCasesComponent.cshtml
@@ -22,10 +22,35 @@
     <span id="checkbox-initially-checked-value">@checkboxInitiallyCheckedValue</span>
 </p>
 
+<h2>Select</h2>
+<p>
+    <select id="select-box" @bind(selectValue)>
+        <option value=@SelectableValue.First>First choice</option>
+        <option value=@SelectableValue.Second>Second choice</option>
+        <option value=@SelectableValue.Third>Third choice</option>
+        @if (includeFourthOption)
+        {
+            <option value=@SelectableValue.Fourth>Fourth choice</option>    
+        }
+    </select>
+    <span id="select-box-value">@selectValue</span>
+    <button id="select-box-add-option" @onclick(AddAndSelectNewSelectOption)>Add and select new item</button>
+</p>
+
 @functions {
     string textboxInitiallyBlankValue = null;
     string textboxInitiallyPopulatedValue = "Hello";
 
     bool checkboxInitiallyUncheckedValue = false;
     bool checkboxInitiallyCheckedValue = true;
+
+    bool includeFourthOption = false;
+    enum SelectableValue { First, Second, Third, Fourth }
+    SelectableValue selectValue = SelectableValue.Second;
+
+    void AddAndSelectNewSelectOption()
+    {
+        includeFourthOption = true;
+        selectValue = SelectableValue.Fourth;
+    }
 }