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

772 set href as a required prop and as the default value for children #776

Merged
merged 18 commits into from
Mar 17, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased]
### Changed
- [#766](https://github.com/plotly/dash-core-components/pull/766) Update from React 16.8.6 to 16.13.0

- [#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.

## [1.8.1] -2020-02-27
### Added
Expand Down
8 changes: 6 additions & 2 deletions src/components/Link.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import PropTypes from 'prop-types';

import React, {Component} from 'react';

import {isNil} from 'ramda';

/*
* event polyfill for IE
* https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent
Expand Down Expand Up @@ -71,7 +73,9 @@ export default class Link extends Component {
href={href}
onClick={e => this.updateLocation(e)}
>
{this.props.children}
{isNil(this.props.children)
? this.props.href
: this.props.children}
Copy link
Collaborator

Choose a reason for hiding this comment

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

While we're golfing - add children to the destructuring of this.props above (href is already there) and this collapses to just {isNil(children) ? href : children}

That's a nicer pattern anyway, as it shows up front what props get used during the render.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense, destructuring does look cleaner. Updated in 065c462.

</a>
);
}
Expand All @@ -87,7 +91,7 @@ Link.propTypes = {
/**
* The URL of a linked resource.
*/
href: PropTypes.string,
href: PropTypes.string.isRequired,
/**
* Controls whether or not the page will refresh when the link is clicked
*/
Expand Down
32 changes: 32 additions & 0 deletions tests/integration/link/test_link_children.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import pytest
import dash
from dash.dependencies import Input, Output
import dash_core_components as dcc
import dash_html_components as html


@pytest.mark.DCC776
def test_lich001_default(dash_dcc):
app = dash.Dash(__name__)
app.layout = html.Div(
[
dcc.Link(id="link1", href="/page-1"),
dcc.Location(id="url", refresh=False),
html.Div(id="content")
]
)
@app.callback(Output("content", "children"), [Input("link1", "children")])
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this callback and content Div do anything in this test?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add another simple test checking that children are indeed children when defined

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added in 5eac66e.

def display_children(children):
return children

dash_dcc.start_server(app)

href_as_children = dash_dcc.driver.execute_script(
'''
return document.getElementById("link1").text;
'''
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

dash_dcc.wait_for_text_to_equal("#link1", "/page-1")

Copy link
Collaborator Author

@HammadTheOne HammadTheOne Mar 14, 2020

Choose a reason for hiding this comment

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

Added in 4537402.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That statement replaces the whole href_as_children = ... and assert href_as_children == ...

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'm guessing that method acts as a wrapper of sorts to check the text? Interesting. Updated in da999d5.


dash_dcc.wait_for_text_to_equal("#link1", "/page-1")

assert href_as_children == "/page-1"