Skip to content

Commit

Permalink
Allow onClick on next/link component's child (#4474)
Browse files Browse the repository at this point in the history
Allow `onClick` on `next/link` child. This should not be a breaking change, but it's a very useful feature. Real-life use cases include: analytics or closing menu on navigation, and other.

- [x] allow optional `onClick` on `next/link` component's child
- [x] call original `child.props.onClick(e)` before `this.linkClicked(e)`
- [x] add integration tests
- [x] cancel the navigation if `e.defaultPrevented === true`

Fixes #1490
  • Loading branch information
Michał Miszczyszyn authored and timneutkens committed May 27, 2018
1 parent 832c494 commit 6692252
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 1 deletion.
9 changes: 8 additions & 1 deletion lib/link.js
Expand Up @@ -127,7 +127,14 @@ export default class Link extends Component {
// This will return the first child, if multiple are provided it will throw an error
const child = Children.only(children)
const props = {
onClick: this.linkClicked
onClick: (e) => {
if (child.props && typeof child.props.onClick === 'function') {
child.props.onClick(e)
}
if (!e.defaultPrevented) {
this.linkClicked(e)
}
}
}

// If child is an <a> tag and doesn't have a href attribute, or if the 'passHref' property is
Expand Down
1 change: 1 addition & 0 deletions test/integration/basic/pages/nav/index.js
Expand Up @@ -38,6 +38,7 @@ export default class extends Component {
<Link href='/nav/as-path' as='/as/path'><a id='as-path-link' style={linkStyle}>As Path</a></Link>
<Link href='/nav/as-path'><a id='as-path-link-no-as' style={linkStyle}>As Path (No as)</a></Link>
<Link href='/nav/as-path-using-router'><a id='as-path-using-router-link' style={linkStyle}>As Path (Using Router)</a></Link>
<Link href='/nav/on-click'><a id='on-click-link' style={linkStyle}>A element with onClick</a></Link>
<button
onClick={() => this.visitQueryStringPage()}
style={linkStyle}
Expand Down
27 changes: 27 additions & 0 deletions test/integration/basic/pages/nav/on-click.js
@@ -0,0 +1,27 @@
import { Component } from 'react'
import Link from 'next/link'

let count = 0

export default class OnClick extends Component {
static getInitialProps ({ res }) {
if (res) return { count: 0 }
count += 1

return { count }
}

render () {
return (
<div id='on-click-page'>
<Link href='/nav/on-click'>
<a id='on-click-link' onClick={() => ++count}>Self Reload</a>
</Link>
<Link href='/nav/on-click'>
<a id='on-click-link-prevent-default' onClick={(e) => { e.preventDefault(); ++count }}>Self Reload</a>
</Link>
<p>COUNT: {this.props.count}</p>
</div>
)
}
}
58 changes: 58 additions & 0 deletions test/integration/basic/test/client-navigation.js
Expand Up @@ -159,6 +159,64 @@ export default (context, render) => {
})
})

describe('with onClick action', () => {
it('should reload the page and perform additional action', async () => {
const browser = await webdriver(context.appPort, '/nav/on-click')
const defaultCount = await browser.elementByCss('p').text()
expect(defaultCount).toBe('COUNT: 0')

const countAfterClicked = await browser
.elementByCss('#on-click-link').click()
.elementByCss('p').text()

// counts (one click + onClick handler)
expect(countAfterClicked).toBe('COUNT: 2')
browser.close()
})

it('should not reload if default was prevented', async () => {
const browser = await webdriver(context.appPort, '/nav/on-click')
const defaultCount = await browser.elementByCss('p').text()
expect(defaultCount).toBe('COUNT: 0')

const countAfterClicked = await browser
.elementByCss('#on-click-link-prevent-default').click()
.elementByCss('p').text()

// counter is increased but there was no reload
expect(countAfterClicked).toBe('COUNT: 0')

const countAfterClickedAndReloaded = await browser
.elementByCss('#on-click-link').click() // +2
.elementByCss('p').text()

// counts (onClick handler, no reload)
expect(countAfterClickedAndReloaded).toBe('COUNT: 3')
browser.close()
})

it('should always replace the state and perform additional action', async () => {
const browser = await webdriver(context.appPort, '/nav')

const countAfterClicked = await browser
.elementByCss('#on-click-link').click() // 1
.waitForElementByCss('#on-click-page')
.elementByCss('#on-click-link').click() // 3
.elementByCss('#on-click-link').click() // 5
.elementByCss('p').text()

// counts (page change + two clicks + onClick handler)
expect(countAfterClicked).toBe('COUNT: 5')

// Since we replace the state, back button would simply go us back to /nav
await browser
.back()
.waitForElementByCss('.nav-home')

browser.close()
})
})

describe('with hash changes', () => {
describe('when hash change via Link', () => {
it('should not run getInitialProps', async () => {
Expand Down

0 comments on commit 6692252

Please sign in to comment.