Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Added click modifiers, target prop #767

Merged
merged 13 commits into from
Mar 19, 2020
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- [#766](https://github.com/plotly/dash-core-components/pull/766) Update from React 16.8.6 to 16.13.0
- [#768](https://github.com/plotly/dash-core-components/pull/768) Added title property to dcc.Link
- [#776](https://github.com/plotly/dash-core-components/pull/776) Update dcc.Link to set href as children if children not defined. Makes href a required prop as well.
- [#767](https://github.com/plotly/dash-core-components/pull/767) Updated dcc.Link to respond to click modifiers.

## [1.8.1] -2020-02-27
### Added
Expand Down
12 changes: 12 additions & 0 deletions src/components/Link.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ export default class Link extends Component {
}

updateLocation(e) {
const hasModifiers = e.metaKey || e.shiftKey || e.altKey || e.ctrlKey;

if (hasModifiers || e.button !== 1) {
return;
}
// prevent anchor from updating location
e.preventDefault();
const {href, refresh} = this.props;
Expand All @@ -64,6 +69,7 @@ export default class Link extends Component {
loading_state,
children,
title,
external,
} = this.props;
/*
* ideally, we would use cloneElement however
Expand All @@ -81,6 +87,7 @@ export default class Link extends Component {
href={href}
onClick={e => this.updateLocation(e)}
title={title}
target={external ? '_blank' : '_self'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that I'd prefer that we just stick to the HTML convention here instead of defining a new API. That way, this component is more or less 1-1 with html.A. So, just let users pass through target directly rather than defining a new external property.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen a couple of other link components define external as a separate property, but I think you raise a good point, for both maintaining parity with html.A and offering a bit more freedom for the user to decide what to target.

Changed in 6514c83.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that I'd prefer that we just stick to the HTML convention here

I agree @chriddyp - cc @Marc-Andre-Rivet - we had a similar discussion a couple of days ago, also because there are other values of target that still matter.

>
{isNil(children) ? href : children}
</a>
Expand Down Expand Up @@ -116,6 +123,10 @@ Link.propTypes = {
* information.
*/
title: PropTypes.string,
/**
* Opens the link in a new tab.
*/
external: PropTypes.bool,
/**
* The children of this component
*/
Expand All @@ -141,4 +152,5 @@ Link.propTypes = {

Link.defaultProps = {
refresh: false,
external: false,
};