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

Issue148 - 0 children regression in dash==0.40.0 #150

Merged
merged 6 commits into from
Apr 11, 2019
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]
### Fixed
- Fix regression for `children=0` case [#148](https://github.com/plotly/dash-renderer/issues/148)

## [0.22.0] - 2019-04-10
### Added
- Added support for clientside callbacks [#143](https://github.com/plotly/dash-renderer/pull/143)
Expand Down
2 changes: 1 addition & 1 deletion src/TreeContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const createContainer = component => isSimpleComponent(component) ?

class TreeContainer extends Component {
getChildren(components) {
if (!components) {
if (isNil(components)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

0 is falsy..

return null;
}

Expand Down
28 changes: 27 additions & 1 deletion tests/test_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def request_queue_assertions(

if expected_length is not None:
self.assertEqual(len(request_queue), expected_length)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh noes, trailing whitespace courtesy of @chriddyp ??? 😅

Copy link
Member

Choose a reason for hiding this comment

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

no way!! 🙊 🙊 🙊


def test_initial_state(self):
app = Dash(__name__)
app.layout = html.Div([
Expand Down Expand Up @@ -475,6 +475,32 @@ def test_initial_state(self):

assert_clean_console(self)

def test_array_of_falsy_child(self):
app = Dash(__name__)
app.layout = html.Div(id='nully-wrapper', children=[0])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This worked and needs to keep working

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice. I'd call this falsy though, not nully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point


self.startServer(app)

self.wait_for_element_by_css_selector('#nully-wrapper')
wrapper = self.driver.find_element_by_id('nully-wrapper')

self.assertEqual(wrapper.get_attribute('innerHTML'), '0')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't this whole section just be self.wait_for_text_to_equal('#nully-wrapper', '0')?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum.. wasn't aware of that one. Looks like wait_for_text_to_equal uses value and that would mean it's only meant to be used with <input /> elements.

Copy link
Member

Choose a reason for hiding this comment

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

value or .text:

(str(self.wait_for_element_by_css_selector(selector).text)

Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Apr 11, 2019

Choose a reason for hiding this comment

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

Embarrassing. I can't read code it seems. Will update to use suggested.


assert_clean_console(self)

def test_of_falsy_child(self):
app = Dash(__name__)
app.layout = html.Div(id='nully-wrapper', children=0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the regression case

Copy link
Contributor

Choose a reason for hiding this comment

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

too late for the comment, but this should be merged with the test_initial_state.


self.startServer(app)

self.wait_for_element_by_css_selector('#nully-wrapper')
wrapper = self.driver.find_element_by_id('nully-wrapper')

self.assertEqual(wrapper.get_attribute('innerHTML'), '0')

assert_clean_console(self)

def test_simple_callback(self):
app = Dash(__name__)
app.layout = html.Div([
Expand Down