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

Confirmation modal #211

Merged
merged 17 commits into from
Jul 18, 2018
Merged
Show file tree
Hide file tree
Changes from 13 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: 2 additions & 0 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ machine:

dependencies:
override:
- pip install setuptools --upgrade
- pip install virtualenv
- pip install tox
- npm install -g eslint
- npm install --ignore-scripts
Expand Down
75 changes: 75 additions & 0 deletions src/components/ConfirmDialog.react.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import PropTypes from 'prop-types';
import {Component} from 'react';

/**
* ConfirmDialog is used to display the browser's native "confirm" modal,
* with an optional message and two buttons ("OK" and "Cancel").
* This ConfirmDialog can be used in conjunction with buttons when the user
* is performing an action that should require an extra step of verification.
*/
export default class ConfirmDialog extends Component {

constructor(props) {
super(props);
}

componentDidUpdate() {
const { displayed, message, setProps, cancel_n_clicks, submit_n_clicks, n_clicks } = this.props;

if (displayed) {
new Promise(resolve => resolve(window.confirm(message))).then(result => setProps({
n_clicks: n_clicks + 1,
n_clicks_timestamp: Date.now(),
cancel_n_clicks: !result ? cancel_n_clicks + 1 : cancel_n_clicks,
submit_n_clicks: result ? submit_n_clicks + 1: submit_n_clicks,
displayed: false,
}));
}
}

render() {
return null;
}
}

ConfirmDialog.defaultProps = {
n_clicks: 0,
n_clicks_timestamp: -1,
submit_n_clicks: 0,
cancel_n_clicks: 0,
};

ConfirmDialog.propTypes = {
id: PropTypes.string,

/**
* Message to show in the popup.
*/
message: PropTypes.string,

/**
* Number of times the modal was submited or canceled.
*/
n_clicks: PropTypes.number,
Copy link
Member

Choose a reason for hiding this comment

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

Given that we have submit_n_clicks and cancel_n_clicks, is there a use case for the general n_clicks?

/**
* Last timestamp the popup was clicked.
*/
n_clicks_timestamp: PropTypes.number,
/**
* Number of times the submit was clicked
*/
submit_n_clicks: PropTypes.number,
/**
* Number of times the popup was canceled.
*/
cancel_n_clicks: PropTypes.number,
/**
* Set to true to send the ConfirmDialog.
*/
displayed: PropTypes.bool,
Copy link
Member

Choose a reason for hiding this comment

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

I think that I thought that displayed would take the place of send_confirm:

@app.callback(Output('confirm', 'displayed'), [Input('my-button', 'n_clicks'])
def display_modal(n_clicks):
    return boolean(n_clicks)

Does that work? Or am I missing a functional difference between displayed and send_confirm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Send_confirm is for activating the modal, it get sets to false after fire, displayed was for telling if the confirmation was currently showing. I just changed for just displayed and it works the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think a good rule to keep in mind here and with most functional/React style coding is that prop names should not be verbs, especially not imperative ones like send but rather descriptions of state like displayed :)


/**
* Dash-assigned callback that gets fired when the value changes.
*/
setProps: PropTypes.func
};
84 changes: 84 additions & 0 deletions src/components/ConfirmDialogProvider.react.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import React from 'react';
import PropTypes from 'prop-types';

import ConfirmDialog from './ConfirmDialog.react'



/**
* Wrap children onClick to send a confirmation dialog.
* You can add a button directly as a children:
* `dcc.ConfirmDialogProvider(html.Button('click me', id='btn'), id='confirm')`
Copy link
Member

Choose a reason for hiding this comment

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

Our users probably won't know what onClick means, so let's change this to:

A wrapper component that will display a confirmation dialog when its child component has been clicked on.
For example:

dcc.ConfirmDialogProvider(
    children=html.Button('Click Me'),
    message='Danger - Are you sure you want to continue?',
    id='confirm'
)

*/
export default class ConfirmDialogProvider extends React.Component {
render() {
const { id, setProps, children } = this.props;

// Will lose the previous onClick of the child
const wrapClick = (child) => React.cloneElement(child, {onClick: () =>
Copy link
Member

Choose a reason for hiding this comment

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

Nice, this seems like a good solution.

{
setProps({
displayed: true
});
}
});

const realChild = children.props
? children.props.children
: children.map(e => e.props.children);

return (
<div id={id}>
{
realChild && realChild.length
? realChild.map(wrapClick)
: wrapClick(realChild)
}
<ConfirmDialog {...this.props}/>
</div>
)
}
};

ConfirmDialogProvider.defaultProps = {
n_clicks: 0,
n_clicks_timestamp: -1,
submit_n_clicks: 0,
cancel_n_clicks: 0,
};

ConfirmDialogProvider.propTypes = {
id: PropTypes.string,

/**
* Message to show in the popup.
*/
message: PropTypes.string,

/**
* Number of times the modal was submited or canceled.
*/
n_clicks: PropTypes.number,
Copy link
Member

Choose a reason for hiding this comment

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

I'm still wondering if there is a use case for this property given that we have submit_n_clicks and cancel_n_clicks. Thoughts @plotly/dash ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I add the timestamps for submit and cancel, I don't see one as I was only using it to know which one was clicked in the test and it wasn't a real use case. I will remove it.

/**
* Last timestamp the popup was clicked.
*/
n_clicks_timestamp: PropTypes.number,
/**
* Number of times the submit was clicked
Copy link
Member

Choose a reason for hiding this comment

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

Number of times the submit button was clicked

*/
submit_n_clicks: PropTypes.number,
/**
* Number of times the popup was canceled.
*/
cancel_n_clicks: PropTypes.number,
Copy link
Member

Choose a reason for hiding this comment

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

Can we add submit_n_clicks_timestamp and cancel_n_clicks_timestamp here and in the ConfirmDialog component?

/**
* Is the modal currently displayed.
*/
displayed: PropTypes.bool,

/**
* Dash-assigned callback that gets fired when the value changes.
*/
setProps: PropTypes.func,
children: PropTypes.any,
};
4 changes: 4 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@ import Textarea from './components/Textarea.react';
import DatePickerSingle from './components/DatePickerSingle.react';
import DatePickerRange from './components/DatePickerRange.react';
import Upload from './components/Upload.react';
import ConfirmDialog from './components/ConfirmDialog.react';
import ConfirmDialogProvider from './components/ConfirmDialogProvider.react'

export {
Checklist,
ConfirmDialog,
ConfirmDialogProvider,
Dropdown,
Graph,
Input,
Expand Down
44 changes: 41 additions & 3 deletions test/IntegrationTests.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,15 @@
import time
import unittest
import percy
import threading
import platform
import flask
import requests

from selenium import webdriver
from selenium.webdriver.chrome.options import Options


class IntegrationTests(unittest.TestCase):

@classmethod
Expand Down Expand Up @@ -34,10 +40,19 @@ def setUp(self):

def tearDown(self):
time.sleep(3)
self.server_process.terminate()
if platform.system() == 'Windows':
requests.get('http://localhost:8050/stop')
else:
self.server_process.terminate()
time.sleep(3)

def startServer(self, app):
"""

:param app:
:type app: dash.Dash
:return:
"""
if 'DASH_TEST_PROCESSES' in os.environ:
processes = int(os.environ['DASH_TEST_PROCESSES'])
else:
Expand All @@ -52,9 +67,32 @@ def run():
processes=processes
)

def run_windows():
app.scripts.config.serve_locally = True
app.css.config.serve_locally = True

@app.server.route('/stop')
def _stop_server_windows():
stopper = flask.request.environ['werkzeug.server.shutdown']
stopper()
return 'stop'

app.run_server(
port=8050,
debug=False,
threaded=True
)

# Run on a separate process so that it doesn't block
self.server_process = multiprocessing.Process(target=run)
self.server_process.start()

system = platform.system()
if system == 'Windows':
# multiprocess can't pickle an inner func on windows (closure are not serializable by default on windows)
self.server_thread = threading.Thread(target=run_windows)
self.server_thread.start()
else:
self.server_process = multiprocessing.Process(target=run)
self.server_process.start()
time.sleep(5)

# Visit the dash page
Expand Down
68 changes: 66 additions & 2 deletions test/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os
import sys
import time
import multiprocessing
import pandas as pd

import dash
Expand All @@ -15,8 +16,9 @@
from selenium import webdriver
from selenium.webdriver.common.keys import Keys
from selenium.common.exceptions import InvalidElementStateException
import time

from textwrap import dedent

try:
from urlparse import urlparse
except ImportError:
Expand Down Expand Up @@ -366,7 +368,6 @@ def test_gallery(self):

self.snapshot('gallery - text input')


def test_location_link(self):
app = dash.Dash(__name__)

Expand Down Expand Up @@ -528,3 +529,66 @@ def update_graph(n_clicks):
button.click()
time.sleep(2)
self.snapshot('candlestick - 2 click')

def _test_confirm(self, app, test_name):
count = multiprocessing.Value('i', 0)

@app.callback(Output('confirmed', 'children'),
[Input('confirm', 'n_clicks'),
Input('confirm', 'submit_n_clicks'),
Input('confirm', 'cancel_n_clicks')])
def _on_confirmed(n_clicks, submit_n_clicks, cancel_n_clicks):
if not n_clicks:
return ''
count.value = n_clicks
if n_clicks == 1:
self.assertEqual(1, submit_n_clicks)
return 'confirmed'
elif n_clicks == 2:
self.assertEqual(1, cancel_n_clicks)
return 'canceled'

self.startServer(app)
self.snapshot(test_name + ' -> initial')
button = self.wait_for_element_by_css_selector('#button')

button.click()
time.sleep(1)
self.driver.switch_to.alert.accept()
Copy link
Member

Choose a reason for hiding this comment

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

🎉 very nice!

self.wait_for_text_to_equal('#confirmed', 'confirmed')
self.snapshot(test_name + ' -> confirmed')

button.click()
time.sleep(0.5)
self.driver.switch_to.alert.dismiss()
time.sleep(0.5)
self.wait_for_text_to_equal('#confirmed', 'canceled')
self.snapshot(test_name + ' -> canceled')

self.assertEqual(2, count.value, 'Expected 2 callback but got ' + str(count.value))

def test_confirm(self):
app = dash.Dash(__name__)

app.layout = html.Div([
html.Button(id='button', children='Send confirm', n_clicks=0),
dcc.ConfirmDialog(id='confirm', message='Please confirm.'),
html.Div(id='confirmed')
])

@app.callback(Output('confirm', 'displayed'), [Input('button', 'n_clicks')])
def on_click_confirm(n_clicks):
if n_clicks:
return True

self._test_confirm(app, 'ConfirmDialog')

def test_confirm_dialog_provider(self):
app = dash.Dash(__name__)

app.layout = html.Div([
dcc.ConfirmDialogProvider(html.Button('click me', id='button'), id='confirm', message='Please confirm.'),
html.Div(id='confirmed')
])

self._test_confirm(app, 'ConfirmDialogProvider')