From 3fd1953143ac6243d9f35f0b339394bb70b168be Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Fri, 12 Jul 2019 16:13:36 -0400 Subject: [PATCH 1/5] Fix all the templates [skip ci] Thanks a lot, GitHub... --- .github/ISSUE_TEMPLATE/------question.md | 82 -------------- .github/ISSUE_TEMPLATE/---bug.md | 102 ------------------ .../---feature-or-enhancement.md | 71 ------------ .github/ISSUE_TEMPLATE/Bug_report.md | 102 ------------------ .github/ISSUE_TEMPLATE/{--bug.md => bug.md} | 2 +- ...e_request.md => feature-or-enhancement.md} | 2 +- .../{-----question.md => question.md} | 2 +- 7 files changed, 3 insertions(+), 360 deletions(-) delete mode 100644 .github/ISSUE_TEMPLATE/------question.md delete mode 100644 .github/ISSUE_TEMPLATE/---bug.md delete mode 100644 .github/ISSUE_TEMPLATE/---feature-or-enhancement.md delete mode 100644 .github/ISSUE_TEMPLATE/Bug_report.md rename .github/ISSUE_TEMPLATE/{--bug.md => bug.md} (99%) rename .github/ISSUE_TEMPLATE/{Feature_request.md => feature-or-enhancement.md} (98%) rename .github/ISSUE_TEMPLATE/{-----question.md => question.md} (98%) diff --git a/.github/ISSUE_TEMPLATE/------question.md b/.github/ISSUE_TEMPLATE/------question.md deleted file mode 100644 index fb540b98..00000000 --- a/.github/ISSUE_TEMPLATE/------question.md +++ /dev/null @@ -1,82 +0,0 @@ ---- -name: "\U0001F64Bโ€โ™€๏ธ Question" -about: Ask a question about Mithril -title: '' -labels: question -assignees: '' - ---- - - - -**Mithril version:** - - -**Browser and OS:** - - -**Project:** - -## Code - -```javascript -// Code -``` - -## Expected Behavior - - -## Current Behavior - - -## Steps to Reproduce - -1. -2. -3. -4. - -## Context - diff --git a/.github/ISSUE_TEMPLATE/---bug.md b/.github/ISSUE_TEMPLATE/---bug.md deleted file mode 100644 index d0b4af49..00000000 --- a/.github/ISSUE_TEMPLATE/---bug.md +++ /dev/null @@ -1,102 +0,0 @@ ---- -name: "\U0001F41B Bug" -about: Report a bug in Mithril -title: '' -labels: bug -assignees: isiahmeadows - ---- - - - -**Mithril version:** - - -**Browser and OS:** - - -**Project:** - -## Code - -```javascript -// Code -``` - -## Steps to Reproduce - -1. -2. -3. -4. - -## Expected Behavior - - -## Current Behavior - - -## Context - diff --git a/.github/ISSUE_TEMPLATE/---feature-or-enhancement.md b/.github/ISSUE_TEMPLATE/---feature-or-enhancement.md deleted file mode 100644 index 1e69d383..00000000 --- a/.github/ISSUE_TEMPLATE/---feature-or-enhancement.md +++ /dev/null @@ -1,71 +0,0 @@ ---- -name: "\U0001F680 Feature or Enhancement" -about: Suggest an idea or feature for Mithril -title: '' -labels: enhancement -assignees: isiahmeadows - ---- - - - -**Mithril version:** - - -**Browser and OS:** - - -**Project:** - - -**Is this something you're interested in implementing yourself?** - -### Description - - -### Why - - -### Possible Implementation - - -### Open Questions - diff --git a/.github/ISSUE_TEMPLATE/Bug_report.md b/.github/ISSUE_TEMPLATE/Bug_report.md deleted file mode 100644 index 843d1e1c..00000000 --- a/.github/ISSUE_TEMPLATE/Bug_report.md +++ /dev/null @@ -1,102 +0,0 @@ ---- -name: "\U0001F41BBug report" -about: Report a bug in Mithril -title: '' -labels: bug -assignees: isiahmeadows - ---- - - - -**Mithril version:** - - -**Browser and OS:** - - -**Project:** - -## Code - -```javascript -// Code -``` - -## Steps to Reproduce - -1. -2. -3. -4. - -## Expected Behavior - - -## Current Behavior - - -## Context - diff --git a/.github/ISSUE_TEMPLATE/--bug.md b/.github/ISSUE_TEMPLATE/bug.md similarity index 99% rename from .github/ISSUE_TEMPLATE/--bug.md rename to .github/ISSUE_TEMPLATE/bug.md index 2a248ee6..219dac12 100644 --- a/.github/ISSUE_TEMPLATE/--bug.md +++ b/.github/ISSUE_TEMPLATE/bug.md @@ -1,5 +1,5 @@ --- -name: "\U0001F41BBug" +name: "๐Ÿ› Bug" about: Report a bug in Mithril title: '' labels: bug diff --git a/.github/ISSUE_TEMPLATE/Feature_request.md b/.github/ISSUE_TEMPLATE/feature-or-enhancement.md similarity index 98% rename from .github/ISSUE_TEMPLATE/Feature_request.md rename to .github/ISSUE_TEMPLATE/feature-or-enhancement.md index ae840348..873905a7 100644 --- a/.github/ISSUE_TEMPLATE/Feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature-or-enhancement.md @@ -1,5 +1,5 @@ --- -name: "\U0001F680Feature or Enhancement" +name: "๐Ÿš€ Feature or Enhancement" about: Suggest an idea or feature for Mithril title: '' labels: enhancement diff --git a/.github/ISSUE_TEMPLATE/-----question.md b/.github/ISSUE_TEMPLATE/question.md similarity index 98% rename from .github/ISSUE_TEMPLATE/-----question.md rename to .github/ISSUE_TEMPLATE/question.md index 6320a5c2..09ea06b7 100644 --- a/.github/ISSUE_TEMPLATE/-----question.md +++ b/.github/ISSUE_TEMPLATE/question.md @@ -1,5 +1,5 @@ --- -name: "\U0001F64Bโ€โ™€๏ธQuestion" +name: "๐Ÿ™‹โ€โ™€๏ธ Question" about: Ask a question about Mithril title: '' labels: question From 6e57a0691ce295c3ee51daf6d2b055b38cb04b7c Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Fri, 12 Jul 2019 16:27:24 -0400 Subject: [PATCH 2/5] Fix #2470 (#2471) --- api/router.js | 8 +++--- api/tests/test-router.js | 58 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/api/router.js b/api/router.js index 365399cf..d224b4a6 100644 --- a/api/router.js +++ b/api/router.js @@ -189,9 +189,11 @@ module.exports = function($window, mountRedraw) { // Remove these so they don't get overwritten var attrs = {}, onclick, href assign(attrs, vnode.attrs) - attrs.component = null - attrs.options = null - attrs.key = null + // The first two are internal, but the rest are magic attributes + // that need censored to not screw up rendering. + attrs.component = attrs.options = attrs.key = attrs.oninit = + attrs.oncreate = attrs.onbeforeupdate = attrs.onupdate = + attrs.onbeforeremove = attrs.onremove = null // Do this now so we can get the most current `href` and `disabled`. // Those attributes may also be specified in the selector, and we diff --git a/api/tests/test-router.js b/api/tests/test-router.js index 8bbeed77..cec9c411 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -550,6 +550,64 @@ o.spec("route", function() { o(root.firstChild.firstChild.nodeValue).equals("text") }) + o("route.Link keeps magic attributes from being double-called", function() { + $window = browserMock(env) + var render = coreRenderer($window) + route = apiRouter(null, null) + route.prefix = prefix + root = $window.document.body + + var oninit = o.spy() + var oncreate = o.spy() + var onbeforeupdate = o.spy() + var onupdate = o.spy() + var onbeforeremove = o.spy() + var onremove = o.spy() + + render(root, m(route.Link, { + href: "/test", + oninit: oninit, + oncreate: oncreate, + onbeforeupdate: onbeforeupdate, + onupdate: onupdate, + onbeforeremove: onbeforeremove, + onremove: onremove, + }, "text")) + + o(oninit.callCount).equals(1) + o(oncreate.callCount).equals(1) + o(onbeforeupdate.callCount).equals(0) + o(onupdate.callCount).equals(0) + o(onbeforeremove.callCount).equals(0) + o(onremove.callCount).equals(0) + + render(root, m(route.Link, { + href: "/test", + oninit: oninit, + oncreate: oncreate, + onbeforeupdate: onbeforeupdate, + onupdate: onupdate, + onbeforeremove: onbeforeremove, + onremove: onremove, + }, "text")) + + o(oninit.callCount).equals(1) + o(oncreate.callCount).equals(1) + o(onbeforeupdate.callCount).equals(1) + o(onupdate.callCount).equals(1) + o(onbeforeremove.callCount).equals(0) + o(onremove.callCount).equals(0) + + render(root, []) + + o(oninit.callCount).equals(1) + o(oncreate.callCount).equals(1) + o(onbeforeupdate.callCount).equals(1) + o(onupdate.callCount).equals(1) + o(onbeforeremove.callCount).equals(1) + o(onremove.callCount).equals(1) + }) + o("route.Link can render other tag without routes or dom access", function() { $window = browserMock(env) var render = coreRenderer($window) From c2f269c3b48f02251fbb5c188bb542f2183eea0b Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Tue, 16 Jul 2019 07:16:25 -0400 Subject: [PATCH 3/5] Fix design bug (#2475) --- api/router.js | 4 ++-- api/tests/test-router.js | 4 ++-- docs/route.md | 22 +++++++++++----------- docs/signatures.md | 20 ++++++++++---------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/api/router.js b/api/router.js index d224b4a6..f52d83f4 100644 --- a/api/router.js +++ b/api/router.js @@ -191,14 +191,14 @@ module.exports = function($window, mountRedraw) { assign(attrs, vnode.attrs) // The first two are internal, but the rest are magic attributes // that need censored to not screw up rendering. - attrs.component = attrs.options = attrs.key = attrs.oninit = + attrs.selector = attrs.options = attrs.key = attrs.oninit = attrs.oncreate = attrs.onbeforeupdate = attrs.onupdate = attrs.onbeforeremove = attrs.onremove = null // Do this now so we can get the most current `href` and `disabled`. // Those attributes may also be specified in the selector, and we // should honor that. - var child = m(vnode.attrs.component || "a", attrs, vnode.children) + var child = m(vnode.attrs.selector || "a", attrs, vnode.children) // Let's provide a *right* way to disable a route link, rather than // letting people screw up accessibility on accident. diff --git a/api/tests/test-router.js b/api/tests/test-router.js index cec9c411..56f617ce 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -615,7 +615,7 @@ o.spec("route", function() { route.prefix = prefix root = $window.document.body - render(root, m(route.Link, {component: "button", href: "/test", foo: "bar"}, "text")) + render(root, m(route.Link, {selector: "button", href: "/test", foo: "bar"}, "text")) o(root.childNodes.length).equals(1) o(root.firstChild.nodeName).equals("BUTTON") @@ -635,7 +635,7 @@ o.spec("route", function() { route.prefix = prefix root = $window.document.body - render(root, m(route.Link, {component: "button[href=/test]", foo: "bar"}, "text")) + render(root, m(route.Link, {selector: "button[href=/test]", foo: "bar"}, "text")) o(root.childNodes.length).equals(1) o(root.firstChild.nodeName).equals("BUTTON") diff --git a/docs/route.md b/docs/route.md index de76b688..c825db95 100644 --- a/docs/route.md +++ b/docs/route.md @@ -144,7 +144,7 @@ You can pass other attributes, too, and you can also specify the tag name used. m(m.route.Link, { // Any hyperscript selector is valid here - it's literally passed as the // first parameter to `m`. - component: "span", + selector: "span", options: {replace: true}, href: "/test", disabled: false, @@ -154,7 +154,7 @@ m(m.route.Link, { }, "link name") ``` -Magic attributes used by this component (except `href` and `disabled`) *are* removed while proxying, so you won't have an odd `component="span"` or `options="[object Object]"` attribute show up in your link's DOM node. The above component renders to this hyperscript, assuming the prefix is the default `#!`: +Magic attributes used by this selector (except `href` and `disabled`) *are* removed while proxying, so you won't have an odd `selector="span"` or `options="[object Object]"` attribute show up in your link's DOM node. The above vnode renders to this hyperscript, assuming the prefix is the default `#!`: ```javascript m("span", { @@ -202,15 +202,15 @@ Do note that this doesn't also disable pointer events for you - you have to do t `vnode = m(m.route.Link, attributes, children)` -Argument | Type | Required | Description ----------------------- | ------------------------------------ | -------- | --- -`attributes.href` | `Object` | Yes | The target route to navigate to. -`attributes.component` | `String|Object|Function` | No | This sets the tag name to use. Must be a valid selector for [`m`](hyperscript.md) if given, defaults to `"a"`. -`attributes.options` | `Object` | No | This sets the options passed to [`m.route.set`](#mrouteset). -`attributes.disabled` | `Object` | No | This sets the options passed to [`m.route.set`](#mrouteset). -`attributes` | `Object` | No | Other attributes to apply to the returned vnode may be passed. -`children` | `Array|String|Number|Boolean` | No | Child [vnodes](vnodes.md) for this link. -**returns** | `Vnode` | | A [vnode](vnodes.md). +Argument | Type | Required | Description +--------------------- | ------------------------------------ | -------- | --- +`attributes.href` | `Object` | Yes | The target route to navigate to. +`attributes.selector` | `String|Object|Function` | No | This sets the tag name to use. Must be a valid selector for [`m`](hyperscript.md) if given, defaults to `"a"`. +`attributes.options` | `Object` | No | This sets the options passed to [`m.route.set`](#mrouteset). +`attributes.disabled` | `Object` | No | This sets the options passed to [`m.route.set`](#mrouteset). +`attributes` | `Object` | No | Other attributes to apply to the returned vnode may be passed. +`children` | `Array|String|Number|Boolean` | No | Child [vnodes](vnodes.md) for this link. +**returns** | `Vnode` | | A [vnode](vnodes.md). ##### m.route.param diff --git a/docs/signatures.md b/docs/signatures.md index a2c306ad..801f3b05 100644 --- a/docs/signatures.md +++ b/docs/signatures.md @@ -57,22 +57,22 @@ Functions with multiple arguments are denoted with parenthesis: `(String, Array) ### Component signatures -Components are denoted via calls to `m`, but with the selector argument set to a constant named in the relevant prose: +Components are denoted via calls to `m`, but with the initial selector argument set to a constant named in the relevant prose: `vnode = m(m.route.Link, attributes, children)` -Argument | Type | Required | Description ----------------------- | ------------------------------------ | -------- | --- -`attributes.href` | `Object` | Yes | The target route to navigate to. -`attributes.component` | `String|Object|Function` | No | This sets the tag name to use. Must be a valid selector for [`m`](hyperscript.md) if given, defaults to `"a"`. -`attributes.options` | `Object` | No | This sets the options passed to [`m.route.set`](#mrouteset). -`attributes` | `Object` | No | Other attributes to apply to the returned vnode may be passed. -`children` | `Array|String|Number|Boolean` | No | Child [vnodes](vnodes.md) for this link. -**returns** | `Vnode` | | A [vnode](vnodes.md). +Argument | Type | Required | Description +--------------------- | ------------------------------------ | -------- | --- +`attributes.href` | `Object` | Yes | The target route to navigate to. +`attributes.selector` | `String|Object|Function` | No | This sets the tag name to use. Must be a valid selector for [`m`](hyperscript.md) if given, defaults to `"a"`. +`attributes.options` | `Object` | No | This sets the options passed to [`m.route.set`](#mrouteset). +`attributes` | `Object` | No | Other attributes to apply to the returned vnode may be passed. +`children` | `Array|String|Number|Boolean` | No | Child [vnodes](vnodes.md) for this link. +**returns** | `Vnode` | | A [vnode](vnodes.md). Children here, if specified, are assumed to be able to be written as [splat arguments](#splats), unless otherwise specified in prose. -An element with no sensible children and/or attributes may elect to elide the relevant parameter entirely, so it might look closer to this: +An element with no sensible children and/or attributes may choose to elide the relevant parameter entirely: `vnode = m(Component, attributes)` From c3cca5f8e261cc3c14c283529d27a3836ca0594b Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Tue, 16 Jul 2019 07:49:22 -0400 Subject: [PATCH 4/5] Remove an outdated bit in the docs [skip ci] --- docs/vnodes.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/vnodes.md b/docs/vnodes.md index 8674f029..be1aae01 100644 --- a/docs/vnodes.md +++ b/docs/vnodes.md @@ -76,7 +76,6 @@ Property | Type | Description `state` | `Object?` | An object that is persisted between redraws. It is provided by the core engine when needed. In POJO component vnodes, the `state` inherits prototypically from the component object/class. In class component vnodes it is an instance of the class. In closure components it is the object returned by the closure. `events` | `Object?` | An object that is persisted between redraws and that stores event handlers so that they can be removed using the DOM API. The `events` property is `undefined` if there are no event handlers defined. This property is only used internally by Mithril, do not use or modify it. `instance` | `Object?` | For components, a storage location for the value returned by the `view`. This property is only used internally by Mithril, do not use or modify it. -`skip` | `Boolean` | This property is only used internally by Mithril when diffing keyed lists, do not use or modify it. --- From 4cbcaf2936b86656d564cd1b0111dec7fb70022c Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Tue, 16 Jul 2019 16:03:24 -0400 Subject: [PATCH 5/5] Fix link + docs (#2476) * Fix a copy/paste fail Also, fix some incorrect tests. * Clarify how routes are diffed, improve key + route resolver docs - Add some missing links to route resolvers and single-child keyed fragments, clarify usage around them. - Drive-by: remove a redundant sentence that itself was missing a period. * Actually test for propagation and preventDefault Previously, the mocks were both junk and inaccurate. No wonder my tests were silently failing - they were wrong and not obviously wrong. --- api/router.js | 11 +- api/tests/test-router.js | 138 ++++++++++++++++++++++++- docs/keys.md | 23 ++++- docs/route.md | 17 +++- render/tests/test-event.js | 201 ++++++++++++++++++++----------------- test-utils/domMock.js | 6 +- 6 files changed, 288 insertions(+), 108 deletions(-) diff --git a/api/router.js b/api/router.js index f52d83f4..45a6fe81 100644 --- a/api/router.js +++ b/api/router.js @@ -237,17 +237,18 @@ module.exports = function($window, mountRedraw) { // link target, etc. Nope, this isn't just for blind people. if ( // Skip if `onclick` prevented default - result === false || !e.defaultPrevented && + result !== false && !e.defaultPrevented && // Ignore everything but left clicks (e.button === 0 || e.which === 0 || e.which === 1) && // Let the browser handle `target=_blank`, etc. (!e.currentTarget.target || e.currentTarget.target === "_self") && // No modifier keys !e.ctrlKey && !e.metaKey && !e.shiftKey && !e.altKey - ) return - e.preventDefault() - e.redraw = false - route.set(href, null, options) + ) { + e.preventDefault() + e.redraw = false + route.set(href, null, options) + } } } return child diff --git a/api/tests/test-router.js b/api/tests/test-router.js index 56f617ce..3989e5f6 100644 --- a/api/tests/test-router.js +++ b/api/tests/test-router.js @@ -465,6 +465,7 @@ o.spec("route", function() { o(oninit.callCount).equals(1) root.firstChild.dispatchEvent(e) + throttleMock.fire() // Wrapped to ensure no redraw fired return waitCycles(1).then(function() { @@ -476,6 +477,7 @@ o.spec("route", function() { var e = $window.document.createEvent("MouseEvents") e.initEvent("click", true, true) + e.button = 0 $window.location.href = prefix + "/" route(root, "/", { @@ -496,7 +498,7 @@ o.spec("route", function() { o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) root.firstChild.dispatchEvent(e) - + throttleMock.fire() o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "") + "test") }) @@ -505,6 +507,7 @@ o.spec("route", function() { var e = $window.document.createEvent("MouseEvents") e.initEvent("click", true, true) + e.button = 0 $window.location.href = prefix + "/" route(root, "/", { @@ -728,6 +731,139 @@ o.spec("route", function() { o(root.firstChild.firstChild.nodeValue).equals("text") }) + o("route.Link doesn't redraw on wrong button", function() { + var e = $window.document.createEvent("MouseEvents") + + e.initEvent("click", true, true) + e.button = 10 + + $window.location.href = prefix + "/" + route(root, "/", { + "/" : { + view: lock(function() { + return m(route.Link, {href: "/test"}) + }) + }, + "/test" : { + view : lock(function() { + return m("div") + }) + } + }) + + var slash = prefix[0] === "/" ? "" : "/" + + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + + root.firstChild.dispatchEvent(e) + throttleMock.fire() + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + }) + + o("route.Link doesn't redraw on preventDefault", function() { + var e = $window.document.createEvent("MouseEvents") + + e.initEvent("click", true, true) + e.button = 0 + + $window.location.href = prefix + "/" + route(root, "/", { + "/" : { + view: lock(function() { + return m(route.Link, { + href: "/test", + onclick: function(e) { + e.preventDefault() + } + }) + }) + }, + "/test" : { + view : lock(function() { + return m("div") + }) + } + }) + + var slash = prefix[0] === "/" ? "" : "/" + + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + + root.firstChild.dispatchEvent(e) + throttleMock.fire() + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + }) + + o("route.Link doesn't redraw on preventDefault in handleEvent", function() { + var e = $window.document.createEvent("MouseEvents") + + e.initEvent("click", true, true) + e.button = 0 + + $window.location.href = prefix + "/" + route(root, "/", { + "/" : { + view: lock(function() { + return m(route.Link, { + href: "/test", + onclick: { + handleEvent: function(e) { + e.preventDefault() + } + } + }) + }) + }, + "/test" : { + view : lock(function() { + return m("div") + }) + } + }) + + var slash = prefix[0] === "/" ? "" : "/" + + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + + root.firstChild.dispatchEvent(e) + throttleMock.fire() + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + }) + + o("route.Link doesn't redraw on return false", function() { + var e = $window.document.createEvent("MouseEvents") + + e.initEvent("click", true, true) + e.button = 0 + + $window.location.href = prefix + "/" + route(root, "/", { + "/" : { + view: lock(function() { + return m(route.Link, { + href: "/test", + onclick: function() { + return false + } + }) + }) + }, + "/test" : { + view : lock(function() { + return m("div") + }) + } + }) + + var slash = prefix[0] === "/" ? "" : "/" + + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + + root.firstChild.dispatchEvent(e) + throttleMock.fire() + o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "")) + }) + o("accepts RouteResolver with onmatch that returns Component", function() { var matchCount = 0 var renderCount = 0 diff --git a/docs/keys.md b/docs/keys.md index 6a49de1f..b38ac671 100644 --- a/docs/keys.md +++ b/docs/keys.md @@ -2,6 +2,7 @@ - [What are keys](#what-are-keys) - [How to use](#how-to-use) + - [Single-child keyed fragments](#single-child-keyed-fragments) - [Debugging key related issues](#debugging-key-related-issues) --- @@ -75,7 +76,9 @@ function correctUserList(users) { } ``` -Also, you might want to reinitialize a component. You can use the common pattern of a single-child keyed fragment where you change the key to destroy and reinitialize the element. +#### Single-child keyed fragments + +Sometimes, you might want to reinitialize a component on command. You can use the common pattern of a single-child keyed fragment where you change the key to destroy and reinitialize the element. ```javascript function ResettableToggle() { @@ -96,6 +99,20 @@ function ResettableToggle() { } ``` +You can also bind it to a known identity, for things like item views where you need to fetch a remote resource based on an ID. It's usually simpler than implementing all the logic to diff the ID and re-fetch a resource if it changes. + +```javascript +function Page() { + return { + view: function() { + return m(Layout, [ + [m(ItemView, {key: m.route.param("id")})], + ]) + } + } +} +``` + --- ### Debugging key related issues @@ -120,7 +137,7 @@ users.map(function(u) { If you refactor the code and make a user component, the key must be moved out of the component and put on the component itself, since it is now the immediate child of the array. ```javascript -// AVOID +// AVOID - doesn't work var User = { view: function(vnode) { return m("div", { key: vnode.attrs.user.id }, [ @@ -137,7 +154,7 @@ users.map(function(u) { #### Avoid wrapping keyed elements in arrays -Arrays are [vnodes](vnodes.md), and therefore keyable. You should not wrap arrays around keyed elements +Arrays are [vnodes](vnodes.md), and therefore keyable. You should not wrap arrays around keyed elements outside [single-child keyed fragments](#single-child-keyed-fragments). ```javascript // AVOID diff --git a/docs/route.md b/docs/route.md index c825db95..59a5144f 100644 --- a/docs/route.md +++ b/docs/route.md @@ -243,6 +243,15 @@ As a rule of thumb, RouteResolvers should be in the same file as the `m.route` c `routeResolver = {onmatch, render}` +When using components, you could think of them as special sugar for this route resolver, assuming your component is `Home`: + +```js +var routeResolver = { + onmatch: function() { return Home }, + render: function(vnode) { return [vnode] }, +} +``` + ##### routeResolver.onmatch The `onmatch` hook is called when the router needs to find a component to render. It is called once per router path changes, but not on subsequent redraws while on the same path. It can be used to run logic before a component initializes (for example authentication logic, data preloading, redirection analytics tracking, etc) @@ -266,7 +275,7 @@ If `onmatch` returns a promise that gets rejected, the router redirects back to ##### routeResolver.render -The `render` method is called on every redraw for a matching route. It is similar to the `view` method in components and it exists to simplify [component composition](#wrapping-a-layout-component). +The `render` method is called on every redraw for a matching route. It is similar to the `view` method in components and it exists to simplify [component composition](#wrapping-a-layout-component). It also lets you escape from Mithril's normal behavior of replacing the entire subtree. `vnode = routeResolve.render(vnode)` @@ -276,6 +285,8 @@ Argument | Type | Description `vnode.attrs` | `Object` | A map of URL parameter values **returns** | `Array|Vnode` | The [vnodes](vnodes.md) to be rendered +The `vnode` parameter is just `m(Component, m.route.param())` where `Component` is the resolved component for the route (after `routeResolver.onmatch`) and `m.route.param()` is as documented [here](#mrouteparam). If you omit this method, the default return value is `[vnode]`, wrapped in a fragment so you can use [key parameters](#key-parameter). Combined with a `:key` parameter, it becomes a [single-element keyed fragment](keys.md#single-child-keyed-fragments), since it ends up rendering to something like `[m(Component, {key: m.route.param("key"), ...})]`. + --- #### How it works @@ -352,7 +363,7 @@ m.route(document.body, "/", { }) ``` -Here we specify two routes: `/` and `/page1`, which render their respective components when the user navigates to each URL. By default, the SPA router prefix is `#!` +Here we specify two routes: `/` and `/page1`, which render their respective components when the user navigates to each URL. --- @@ -364,6 +375,8 @@ You can also navigate programmatically, via `m.route.set(route)`. For example, ` When navigating between routes, the router prefix is handled for you. In other words, leave out the hashbang `#!` (or whatever prefix you set `m.route.prefix` to) when linking Mithril routes, including in both `m.route.set` and in `m.route.Link`. +Do note that when navigating between components, the entire subtree is replaced. Use [a route resolver with a `render` method](#routeresolverrender) if you want to just patch the subtree. + --- ### Routing parameters diff --git a/render/tests/test-event.js b/render/tests/test-event.js index 381b4276..b0b00592 100644 --- a/render/tests/test-event.js +++ b/render/tests/test-event.js @@ -16,107 +16,124 @@ o.spec("event", function() { } }) + function eventSpy(fn) { + function spy(e) { + spy.calls.push({ + this: this, type: e.type, + target: e.target, currentTarget: e.currentTarget, + }) + if (fn) return fn.apply(this, arguments) + } + spy.calls = [] + return spy + } + o("handles onclick", function() { - var spy = o.spy() - var div = {tag: "div", attrs: {onclick: spy}} - var e = $window.document.createEvent("MouseEvents") - e.initEvent("click", true, true) - - render(root, [div]) - div.dom.dispatchEvent(e) - - o(spy.callCount).equals(1) - o(spy.this).equals(div.dom) - o(spy.args[0].type).equals("click") - o(spy.args[0].target).equals(div.dom) - o(redraw.callCount).equals(1) - o(redraw.this).equals(undefined) - o(redraw.args.length).equals(0) - o(e.$defaultPrevented).equals(false) - o(e.$propagationStopped).equals(false) - }) - - o("handles onclick returning false", function() { - var spy = o.spy(function () { return false }) - var div = {tag: "div", attrs: {onclick: spy}} - var e = $window.document.createEvent("MouseEvents") - e.initEvent("click", true, true) - - render(root, [div]) - div.dom.dispatchEvent(e) - - o(spy.callCount).equals(1) - o(spy.this).equals(div.dom) - o(spy.args[0].type).equals("click") - o(spy.args[0].target).equals(div.dom) - o(redraw.callCount).equals(1) - o(redraw.this).equals(undefined) - o(redraw.args.length).equals(0) - o(e.$defaultPrevented).equals(true) - o(e.$propagationStopped).equals(true) - }) - - o("handles click EventListener object", function() { - var spy = o.spy() - var listener = {handleEvent: spy} - var div = {tag: "div", attrs: {onclick: listener}} - var e = $window.document.createEvent("MouseEvents") - e.initEvent("click", true, true) - - render(root, [div]) - div.dom.dispatchEvent(e) - - o(spy.callCount).equals(1) - o(spy.this).equals(listener) - o(spy.args[0].type).equals("click") - o(spy.args[0].target).equals(div.dom) - o(redraw.callCount).equals(1) - o(redraw.this).equals(undefined) - o(redraw.args.length).equals(0) - o(e.$defaultPrevented).equals(false) - o(e.$propagationStopped).equals(false) - }) - - o("handles click EventListener object returning false", function() { - var spy = o.spy(function () { return false }) - var listener = {handleEvent: spy} - var div = {tag: "div", attrs: {onclick: listener}} - var e = $window.document.createEvent("MouseEvents") - e.initEvent("click", true, true) - - render(root, [div]) - div.dom.dispatchEvent(e) - - o(spy.callCount).equals(1) - o(spy.this).equals(listener) - o(spy.args[0].type).equals("click") - o(spy.args[0].target).equals(div.dom) - o(redraw.callCount).equals(1) - o(redraw.this).equals(undefined) - o(redraw.args.length).equals(0) - o(e.$defaultPrevented).equals(false) - o(e.$propagationStopped).equals(false) - }) - - o("handles propagated onclick", function() { - var spy = o.spy() - var child = {tag: "div"} - var parent = {tag: "div", attrs: {onclick: spy}, children: [child]} + var spyDiv = eventSpy() + var spyParent = eventSpy() + var div = {tag: "div", attrs: {onclick: spyDiv}} + var parent = {tag: "div", attrs: {onclick: spyParent}, children: [div]} var e = $window.document.createEvent("MouseEvents") e.initEvent("click", true, true) render(root, [parent]) - child.dom.dispatchEvent(e) + div.dom.dispatchEvent(e) - o(spy.callCount).equals(1) - o(spy.this).equals(parent.dom) - o(spy.args[0].type).equals("click") - o(spy.args[0].target).equals(child.dom) + o(spyDiv.calls.length).equals(1) + o(spyDiv.calls[0].this).equals(div.dom) + o(spyDiv.calls[0].type).equals("click") + o(spyDiv.calls[0].target).equals(div.dom) + o(spyDiv.calls[0].currentTarget).equals(div.dom) + o(spyParent.calls.length).equals(1) + o(spyParent.calls[0].this).equals(parent.dom) + o(spyParent.calls[0].type).equals("click") + o(spyParent.calls[0].target).equals(div.dom) + o(spyParent.calls[0].currentTarget).equals(parent.dom) + o(redraw.callCount).equals(2) + o(redraw.this).equals(undefined) + o(redraw.args.length).equals(0) + o(e.defaultPrevented).equals(false) + }) + + o("handles onclick returning false", function() { + var spyDiv = eventSpy(function() { return false }) + var spyParent = eventSpy() + var div = {tag: "div", attrs: {onclick: spyDiv}} + var parent = {tag: "div", attrs: {onclick: spyParent}, children: [div]} + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + + render(root, [parent]) + div.dom.dispatchEvent(e) + + o(spyDiv.calls.length).equals(1) + o(spyDiv.calls[0].this).equals(div.dom) + o(spyDiv.calls[0].type).equals("click") + o(spyDiv.calls[0].target).equals(div.dom) + o(spyDiv.calls[0].currentTarget).equals(div.dom) + o(spyParent.calls.length).equals(0) o(redraw.callCount).equals(1) o(redraw.this).equals(undefined) o(redraw.args.length).equals(0) - o(e.$defaultPrevented).equals(false) - o(e.$propagationStopped).equals(false) + o(e.defaultPrevented).equals(true) + }) + + o("handles click EventListener object", function() { + var spyDiv = eventSpy() + var spyParent = eventSpy() + var listenerDiv = {handleEvent: spyDiv} + var listenerParent = {handleEvent: spyParent} + var div = {tag: "div", attrs: {onclick: listenerDiv}} + var parent = {tag: "div", attrs: {onclick: listenerParent}, children: [div]} + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + + render(root, [parent]) + div.dom.dispatchEvent(e) + + o(spyDiv.calls.length).equals(1) + o(spyDiv.calls[0].this).equals(listenerDiv) + o(spyDiv.calls[0].type).equals("click") + o(spyDiv.calls[0].target).equals(div.dom) + o(spyDiv.calls[0].currentTarget).equals(div.dom) + o(spyParent.calls.length).equals(1) + o(spyParent.calls[0].this).equals(listenerParent) + o(spyParent.calls[0].type).equals("click") + o(spyParent.calls[0].target).equals(div.dom) + o(spyParent.calls[0].currentTarget).equals(parent.dom) + o(redraw.callCount).equals(2) + o(redraw.this).equals(undefined) + o(redraw.args.length).equals(0) + o(e.defaultPrevented).equals(false) + }) + + o("handles click EventListener object returning false", function() { + var spyDiv = eventSpy(function() { return false }) + var spyParent = eventSpy() + var listenerDiv = {handleEvent: spyDiv} + var listenerParent = {handleEvent: spyParent} + var div = {tag: "div", attrs: {onclick: listenerDiv}} + var parent = {tag: "div", attrs: {onclick: listenerParent}, children: [div]} + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + + render(root, [parent]) + div.dom.dispatchEvent(e) + + o(spyDiv.calls.length).equals(1) + o(spyDiv.calls[0].this).equals(listenerDiv) + o(spyDiv.calls[0].type).equals("click") + o(spyDiv.calls[0].target).equals(div.dom) + o(spyDiv.calls[0].currentTarget).equals(div.dom) + o(spyParent.calls.length).equals(1) + o(spyParent.calls[0].this).equals(listenerParent) + o(spyParent.calls[0].type).equals("click") + o(spyParent.calls[0].target).equals(div.dom) + o(spyParent.calls[0].currentTarget).equals(parent.dom) + o(redraw.callCount).equals(2) + o(redraw.this).equals(undefined) + o(redraw.args.length).equals(0) + o(e.defaultPrevented).equals(false) }) o("removes event", function() { diff --git a/test-utils/domMock.js b/test-utils/domMock.js index 87b45ad2..90fe26ab 100644 --- a/test-utils/domMock.js +++ b/test-utils/domMock.js @@ -401,7 +401,7 @@ module.exports = function(options) { e.preventDefault = function() { prevented = true } - Object.defineProperty(e, "$defaultPrevented", { + Object.defineProperty(e, "defaultPrevented", { configurable: true, get: function () { return prevented } }) @@ -409,10 +409,6 @@ module.exports = function(options) { e.stopPropagation = function() { stopped = true } - Object.defineProperty(e, "$propagationStopped", { - configurable: true, - get: function () { return prevented } - }) e.eventPhase = 1 try { for (var i = parents.length - 1; 0 <= i; i--) {