772 set href as a required prop and as the default value for children #776
Conversation
HammadTheOne
requested review from
alexcjohnson and
Marc-Andre-Rivet
as code owners
| ''' | ||
| return document.getElementById("link1").text; | ||
| ''' | ||
| ) |
There was a problem hiding this comment.
dash_dcc.wait_for_text_to_equal("#link1", "/page-1")
There was a problem hiding this comment.
Added in 4537402.
There was a problem hiding this comment.
That statement replaces the whole href_as_children = ... and assert href_as_children == ...
There was a problem hiding this comment.
I'm guessing that method acts as a wrapper of sorts to check the text? Interesting. Updated in da999d5.
Co-Authored-By: alexcjohnson <[email protected]>
src/components/Link.react.js
Outdated
| {this.props.children} | ||
| {isNil(this.props.children) | ||
| ? this.props.href | ||
| : this.props.children} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Makes sense, destructuring does look cleaner. Updated in 065c462.
| html.Div(id="content") | ||
| ] | ||
| ) | ||
| @app.callback(Output("content", "children"), [Input("link1", "children")]) |
There was a problem hiding this comment.
Does this callback and content Div do anything in this test?
There was a problem hiding this comment.
Can you add another simple test checking that children are indeed children when defined
There was a problem hiding this comment.
Added in 5eac66e.
| html.Div(id="content") | ||
| ] | ||
| ) | ||
| @app.callback(Output("content", "children"), [Input("link1", "children")]) |
There was a problem hiding this comment.
Can you add another simple test checking that children are indeed children when defined
This PR addresses #772 and #773. The
hrefprop is marked as required, and if nochildrenprop is defined, thehrefis set as thechildrenby default. As @Marc-Andre-Rivet pointed out, this will simplify usage, and makes more sense for thedcc.Linkcomponent as a whole.Any suggestions on how to better structure the conditional are welcome, would it perhaps be better to set the children directly in the return function (maybe as a ternary operator) instead of as a separate named function?
Closes #772
Closes #773