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

@@ -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.preventDefault()
-					e.redraw = false
+						e.redraw = false
-					route.set(href, null, options)
+						route.set(href, null, options)
+					}
 				}
 			}
 			return child

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

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

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>|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

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 spyDiv = eventSpy()
-		var div = {tag: "div", attrs: {onclick: spy}}
+		var spyParent = eventSpy()
-		var e = $window.document.createEvent("MouseEvents")
+		var div = {tag: "div", attrs: {onclick: spyDiv}}
-		e.initEvent("click", true, true)
+		var parent = {tag: "div", attrs: {onclick: spyParent}, children: [div]}
-
-		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 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(spyDiv.calls.length).equals(1)
-		o(spy.this).equals(parent.dom)
+		o(spyDiv.calls[0].this).equals(div.dom)
-		o(spy.args[0].type).equals("click")
+		o(spyDiv.calls[0].type).equals("click")
-		o(spy.args[0].target).equals(child.dom)
+		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.defaultPrevented).equals(true)
-		o(e.$propagationStopped).equals(false)
+	})
+
+	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() {

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--) {