-
Notifications
You must be signed in to change notification settings - Fork 33
Issue148 - 0
children regression in dash==0.40.0
#150
Conversation
@@ -33,7 +33,7 @@ const createContainer = component => isSimpleComponent(component) ? | |||
|
|||
class TreeContainer extends Component { | |||
getChildren(components) { | |||
if (!components) { | |||
if (isNil(components)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 is falsy..
@@ -475,6 +475,32 @@ def test_initial_state(self): | |||
|
|||
assert_clean_console(self) | |||
|
|||
def test_array_of_nully_child(self): | |||
app = Dash(__name__) | |||
app.layout = html.Div(id='nully-wrapper', children=[0]) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point
|
||
def test_of_nully_child(self): | ||
app = Dash(__name__) | ||
app.layout = html.Div(id='nully-wrapper', children=0) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
.
0
children regression0
children regression in dash==0.40.0
@@ -104,7 +104,7 @@ def request_queue_assertions( | |||
|
|||
if expected_length is not None: | |||
self.assertEqual(len(request_queue), expected_length) | |||
|
There was a problem hiding this comment.
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 ??? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no way!! 🙊 🙊 🙊
tests/test_render.py
Outdated
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') |
There was a problem hiding this comment.
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')
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
value
or .text
:
dash-renderer/tests/test_render.py
Line 63 in 2c0169c
(str(self.wait_for_element_by_css_selector(selector).text) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching & fixing this! Only pedantic comments 💃
FIxes #148.
Found the issue in dash-docs percy deltas in https://percy.io/plotly/dash-docs/builds/1719186
Add test for array case
children=[0]
, this passes with the existing code: 0602a15Add test for non-array case
children=0
, this fails with the existing code: d48e81dFix: b600b8c