Added n_clicks_previous property to all components

[rmarren1]

Description

This gives buttons a n_clicks_previous property to the Button component. You can add this as a State in a callback and compare n_clicks_previous to n_clicks to see if a button has been clicked.

Notes

• Properties n_clicks and n_clicks_previous should be initialized to 0, since if a button has never been pressed before these values default to None (default properties are set by a shallow merge, so when you change the id variables the defaults setting these values to 0 go away). This would cause an error in the following example.
• There are many changes to files when npm install is ran. This request does not include those changes (see npm install #21)

Example (select and de-select all for a checklist component)

@app.callback(Output('checklist', 'values'),
             [Input('selectall-button', 'n_clicks'),
              Input('deselectall-button', 'n_clicks')],
             [State('selectall-button', 'n_clicks_previous'),
              State('deselectall-button', 'n_clicks_previous'),
              State('checklist', 'options')])
def select_all(select, deselect, select_prev, deselect_prev, options):
    values = [o['value'] for o in options]
    if select > select_prev: # The select button was pressed
        return values
    elif deselect > deselect_prev: # The deselect button
        return []
    else: # Should only reach here on page load, so its OK to reset since it should be empty
        return []

[cortexml]

Thank you!! :) Please pull this into the main branch! I've been waiting for this fix for a long time!

[rmarren1]

I pushed this in a separate package to use in deployment now while waiting for a pull. You can pip3 install ipop-components==0.0.2 then import ipop_components as ipop then use ipop.Button(...) to use it now.

[cortexml]

This is great, thanks so much for your help Ryan 💯 😃

[chriddyp]

Thanks for opening! I'm 👍 with this change in principle. I think we should add it to all of the HTML components rather than just the button. I'd also like to see some integration tests with this behaviour.

To add this to all of the HTML components, we'll modify the template component: https://github.com/plotly/dash-html-components/blob/master/scripts/generate-components.js#L99-L119

For integration tests, I'll add the framework in a separate PR and then we can rebase this PR off of master and write an integration test for this particular feature.

@rmarren1 - Let me know if you have to continue working on this PR, otherwise I'll jump in.

[chriddyp]

Alright, integration tests have been added in #38 . So, let's:

1. rebase off of master
2. modify the template component so that n_clicks_previous is added to all of the components
3. right a simple integration test
   again, @rmarren1 let me know if you want me to step in here

[mjkramer]

@rmarren1 Thank you so much for doing this! Just wanted to point out that the ipop-components package isn't working because the latest version on npm is 0.0.1 instead of 0.0.2.

Temporary workaround here. Import that instead of ipop_components until npm is updated (ipop_components must be installed).

Edit: Or duh, one can make ipop_components work by just setting app.scripts.config.serve_locally = True

[rmarren1]

percy changes are likely from here, not sure though it is not letting me see what the changes are.

[chriddyp]

I actually don't quite understand this - how is n_clicks_previous just not equal to n_clicks?

[chriddyp]

setProps is the dash-renderer provided function which will propagate these changes down to Dash's backend and then causes the component / app to re-render. I'm a little wary of calling setProps twice on an onClick handler as I would expect that this would fire the callbacks twice and cause multiple re-renders. In practice, the callback might not be called twice because dash-renderer does some things to trim an excessive number of requests (plotly/dash-renderer#22).

Instead, what if we did something like:

const newProps = {
    n_clicks: props.n_clicks + 1
}
if (newProps.n_clicks > 1) {
    newProps.n_clicks_previous = props.n_clicks_previous + 1;
}
setProps(newProps)

[rmarren1]

I tried something similar, but I found that it has equivalent behavior. When there are two properties in newProps and both of those properties are inputs of a callback, the callback is triggered twice.

For example, setting the innards of generate-component.js to

return (
            <${element}
                onClick={() => {
                    const newProps = {
                        n_clicks: props.n_clicks + 1
                    }
                    if (newProps.n_clicks > 1) {
                        newProps.n_clicks_previous = props.n_clicks_previous + 1
                    }
                    if (props.setProps) props.setProps(newProps);
                    if (props.fireEvent) props.fireEvent({event: 'click'});
                }}
                {...props}
            >
                {props.children}
            </${element}>
);

Generating the components, then testing with the test cases in test_integration.py as

def test_click(self):
        call_count = Value('i', 0)

        app = dash.Dash()
        app.layout = html.Div([
            html.Div(id='container'),
            html.Button('Click', id='button', n_clicks=0, n_clicks_previous=0)
        ])

        output_string = "You have clicked the button {} times." + \
                        "Previously, you have clicked the button {} times."

        @app.callback(Output('container', 'children'),
                      [Input('button', 'n_clicks'),
                       Input('button', 'n_clicks_previous')])
        def update_output(n_clicks, n_clicks_previous):
            call_count.value += 1
            print(n_clicks, n_clicks_previous)
            return output_string.format(n_clicks, n_clicks_previous)

        self.startServer(app)

        self.wait_for_element_by_css_selector('#container')

        self.wait_for_text_to_equal(
            '#container', output_string.format(0, 0))
        self.assertEqual(call_count.value, 1)
        self.snapshot('button initialization')

        self.driver.find_element_by_css_selector('#button').click()

        self.wait_for_text_to_equal(
            '#container', output_string.format(1, 0))
        self.assertEqual(call_count.value, 2)
        self.snapshot('button click one')

        self.driver.find_element_by_css_selector('#button').click()

        self.wait_for_text_to_equal(
            '#container', output_string.format(2, 1))
        self.assertEqual(call_count.value, 3)
        self.snapshot('button click two')

Yields the following testing error (while printing n_clicks and n_clicks_previous with each callback)

py27 runtests: commands[1] | python -m unittest tests.test_dash_html_components
..
----------------------------------------------------------------------
Ran 2 tests in 0.001s

OK
py27 runtests: commands[2] | python -m unittest tests.test_integration
 * Running on http://127.0.0.1:8050/ (Press CTRL+C to quit)
127.0.0.1 - - [03/Apr/2018 21:58:54] "GET / HTTP/1.1" 200 -
127.0.0.1 - - [03/Apr/2018 21:58:54] "GET /_dash-component-suites/dash_renderer/[email protected]?v=0.12.1 HTTP/1.1" 200 -
127.0.0.1 - - [03/Apr/2018 21:58:54] "GET /_dash-component-suites/dash_renderer/[email protected]?v=0.12.1 HTTP/1.1" 200 -
127.0.0.1 - - [03/Apr/2018 21:58:54] "GET /_dash-component-suites/dash_renderer/bundle.js?v=0.12.1 HTTP/1.1" 200 -
127.0.0.1 - - [03/Apr/2018 21:58:54] "GET /_dash-component-suites/dash_html_components/bundle.js?v=0.10.0 HTTP/1.1" 200 -
127.0.0.1 - - [03/Apr/2018 21:58:55] "GET /_dash-layout HTTP/1.1" 200 -
127.0.0.1 - - [03/Apr/2018 21:58:55] "GET /_dash-dependencies HTTP/1.1" 200 -
(0, 0)  # Initialization of the component
127.0.0.1 - - [03/Apr/2018 21:58:55] "POST /_dash-update-component HTTP/1.1" 200 -
127.0.0.1 - - [03/Apr/2018 21:58:55] "GET /favicon.ico HTTP/1.1" 200 -
(1, 0) # First click -- only one property (props.n_clicks) is changed
127.0.0.1 - - [03/Apr/2018 21:58:56] "POST /_dash-update-component HTTP/1.1" 200 -
(2, 1) # Second click -- both props.n_clicks and props.n_clicks_previous are changed
127.0.0.1 - - [03/Apr/2018 21:58:56] "POST /_dash-update-component HTTP/1.1" 200 -
(2, 1) # Callback is triggered again
127.0.0.1 - - [03/Apr/2018 21:58:56] "POST /_dash-update-component HTTP/1.1" 200 -
F
======================================================================
FAIL: test_click (tests.test_integration.Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/test_integration.py", line 98, in test_click
    self.assertEqual(call_count.value, 3)
AssertionError: 4 != 3

----------------------------------------------------------------------
Ran 1 test in 15.905s

FAILED (failures=1)
ERROR: InvocationError for command '/home/ryan/MSK/dash/dash-html-components/.tox/py27/bin/python -m unittest tests.test_integration' (exited with code 1)
_______________________________________________ summary ________________________________________________
ERROR:   py27: commands failed

Thus, it only behaves correctly if n_clicks_previous is a State variable instead of Input, in which case it only matters what value n_clicks_previous has when setProps is called on n_clicks, so I thought what I had was simpler.

[chriddyp]

Looking pretty good! The templating is 👍 and so are the integration and percy screenshot tests. Let's just try out the suggestion in https://github.com/plotly/dash-html-components/pull/37/files#r178873005 to make it a little bit more idiomatically dash/React