WIP Partial for fix #546 - Add more events #718

rynowak · 2018-05-01T05:27:03Z

Work in progress adding more built-in event types. I figure I'll keep working on this for a bit and we can merge whatever's ready before we branch

rynowak · 2018-05-01T05:29:43Z

src/Microsoft.AspNetCore.Blazor.Browser/Rendering/BrowserRendererEventDispatcher.cs

                default:
-                    throw new ArgumentException($"Unsupported value '{eventArgsType}'.", nameof(eventArgsType));
+                    return JsonUtil.Deserialize<UIEventArgs>(eventArgsJson);


Changing this to allow 'unknown' to flow through makes it possible to at least pipe through any event without data.

We might want to consider something in the future that makes this abitrarily extensible by mapping 'unknown' to something with a dictionary or JSON blob. You can imagine that components might use their own bubbling events for notifications - this pattern was used in WPF as well

OR alternatively, we could allow you to do strongly typed things by subclassing UIEventArgs

Yes, that would be an improvement. As part of fixing the non-bubbling events, I'll enable passthrough of unknown event types, mapping them simply to UIEventHandler. We can consider ways of including custom data as a future enhancement.

Just the notifications alone cover a large proportion of scenarios.

BTW I've handled this in #722 so if that PR goes in first, you could revert this change to default.

rynowak · 2018-05-01T05:30:50Z

test/Microsoft.AspNetCore.Blazor.E2ETest/Infrastructure/BrowserFixture.cs

+        public IKeyboard Keyboard => (Browser as IHasInputDevices)?.Keyboard;
+
+        public IMouse Mouse => (Browser as IHasInputDevices)?.Mouse;
+


Will revert this, I ended up not needing it.

rynowak · 2018-05-01T05:30:55Z

test/Microsoft.AspNetCore.Blazor.E2ETest/Infrastructure/BrowserTestBase.cs


        public static ITestOutputHelper Output => _output.Value;

        public BrowserTestBase(BrowserFixture browserFixture, ITestOutputHelper output)
        {
-            _browser.Value = browserFixture.Browser;
-            _logs.Value = browserFixture.Logs;
+            _fixture.Value = browserFixture;


Will revert this, I ended up not needing it.

rynowak · 2018-05-01T05:31:57Z

test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/EventTest.cs

+            // Mouse over the button and then back off
+            var actions = new Actions(Browser)
+                .MoveToElement(input)
+                .MoveToElement(other);


I'm far from a webdriver expert but this seemed to be what they recommend for general interactivity scripting.

We also have the ability to run javascript to trigger events, but I wanted to avoid that until its absolutely necessary because that seems like a 'less good' way to test.

Oh, well, I should have read this earlier. Just realised I've also implemented similar E2E events coverage in #722. Sorry for duplicating your work! I even used the same filename 😞

I guess it would be good to merge what we've done together. Your test has more coverage of the mouse events, whereas mine has more coverage of the bubbling behaviors.

SteveSandersonMS · 2018-05-01T14:39:40Z

src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/EventForDotNet.ts


 export interface UIEventArgs {
  Type: string;
 }

+export interface UIFocusEventArgs extends UIEventArgs {


I don't think this interface needs to be exported, as we're only using the type parameters within this file as a way of checking that the constructor args are of a recognized format.

It wouldn't be harmful to export it either, but if we are doing so, it would be good to export all the others too for consistency.

Ah right, I didn't notice that. Will undo the export until we actually need these types exported

SteveSandersonMS · 2018-05-01T14:46:55Z

test/testapps/BasicTestApp/wwwroot/index.html

+          <option value="BasicTestApp.ComponentRefComponent">Component ref component</option>
+          <option value="BasicTestApp.AfterRenderInteropComponent">After-render interop component</option>
+          <!--<option value="BasicTestApp.RouterTest.Default">Router</option> Excluded because it requires additional setup to work correctly when loaded manually -->
+      </select>


Yes, I guess it's time to stop fighting VS over the indentation here!

The alternative would be to use an .editorsettings - VS should honour that.

…ling.

This change adds tag helpers and defines event types for all of the DOM events we could find. You'll find it much easier now to subcribe to these events. While we did define new event types, we didn't substantially expand the set of data that we serialize for events. We're looking for feedback on what is most critical to have, and looking for contributions adding those data and tests.

rynowak · 2018-05-01T19:03:18Z

@SteveSandersonMS - I added everything from my list.

I'm planning to merge when the CI is green unless you have major feedback. I felt like adding the types was important because you have 4-5 things that are required just to make the type work end to end. I didn't add the data because I don't feel good doing that without tests. This is makes it much easier for someone to do a contribution if they care about a specific event or piece of data.

SteveSandersonMS · 2018-05-01T19:35:01Z

test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/EventTest.cs

+            Assert.Equal("onmousedown,onmouseup,", output.Text);
+        }
+
+        private string[] GetLogLines()


Looks like GetLogLines and TriggerCustomBubblingEvent aren't used, if I'm reading this correctly.

SteveSandersonMS · 2018-05-01T19:36:28Z

@rynowak Excellent - thanks for being so quick with all this! Minor point about possibly unused test code, but don't let it stop you from cherry-picking to release/0.3.0.

rynowak · 2018-05-01T22:59:44Z

6903327

rynowak commented May 1, 2018

View reviewed changes

SteveSandersonMS reviewed May 1, 2018

View reviewed changes

SteveSandersonMS mentioned this pull request May 1, 2018

Support custom events and non-bubbling standard events #722

Merged

rynowak force-pushed the rynowak/additional-events branch from 1c0ba27 to 0398ddb Compare May 1, 2018 15:01

SteveSandersonMS and others added 3 commits May 1, 2018 10:49

Support responding to arbitrary events. E2E coverage of this and bubb…

2d82bf7

…ling.

Rename E2E test files to avoid clash with other PR

158aefe

rynowak force-pushed the rynowak/additional-events branch from 0398ddb to cfe44ef Compare May 1, 2018 19:01

rynowak merged commit 4ae907d into dev May 1, 2018

rynowak deleted the rynowak/additional-events branch May 1, 2018 19:22

SteveSandersonMS reviewed May 1, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP Partial for fix #546 - Add more events #718

WIP Partial for fix #546 - Add more events #718

rynowak commented May 1, 2018 •

edited

Loading

rynowak May 1, 2018

rynowak May 1, 2018

SteveSandersonMS May 1, 2018

SteveSandersonMS May 1, 2018

rynowak May 1, 2018

rynowak May 1, 2018

rynowak May 1, 2018

SteveSandersonMS May 1, 2018

SteveSandersonMS May 1, 2018

rynowak May 1, 2018

SteveSandersonMS May 1, 2018

rynowak May 1, 2018

rynowak commented May 1, 2018

SteveSandersonMS May 1, 2018

SteveSandersonMS commented May 1, 2018

rynowak commented May 1, 2018

		public IKeyboard Keyboard => (Browser as IHasInputDevices)?.Keyboard;

		public IMouse Mouse => (Browser as IHasInputDevices)?.Mouse;

WIP Partial for fix #546 - Add more events #718

WIP Partial for fix #546 - Add more events #718

Conversation

rynowak commented May 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rynowak commented May 1, 2018

Choose a reason for hiding this comment

SteveSandersonMS commented May 1, 2018

rynowak commented May 1, 2018

rynowak commented May 1, 2018 •

edited

Loading