Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistence fix #944

Merged
merged 9 commits into from
Sep 30, 2019
12 changes: 9 additions & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ jobs:
name: ️️🏗️ build core
command: |
. venv/bin/activate && pip install --no-cache-dir --upgrade -e . --progress-bar off && mkdir packages
python setup.py sdist && mv dist/* packages/
cd dash-renderer && renderer build && python setup.py sdist && mv dist/* ../packages/ && cd ..
git clone --depth 1 https://github.com/plotly/dash-core-components.git
cd dash-core-components && npm install --ignore-scripts && npm run build && python setup.py sdist && mv dist/* ../packages/ && cd ..
Expand Down Expand Up @@ -200,10 +199,17 @@ jobs:
- attach_workspace:
at: ~/dash
- run:
name: 🧪 Run Integration Tests
name: ️️🏗️ Install packages
command: |
. venv/bin/activate && cd packages && ls -la
find . -name "*.gz" | xargs pip install --no-cache-dir --ignore-installed && pip list | grep dash && cd ..
find . -name "*.gz" | xargs pip install --no-cache-dir --ignore-installed && cd ..
sed -i '/dash/d' requires-install.txt
pip install --no-cache-dir --ignore-installed .
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI @byronz - the issue I was having in this PR is related to the original one that led to the sed call during build-core: sometimes (seemingly at random) the dash-renderer we prepared in packages was being overwritten by a published one from PyPI - so I used essentially the same solution, but extended to dash-renderer by using the pattern /dash/d rather than /dash-/d.

Also I skipped creating a tarball for dash itself because (a) we already have everything we need here, dash has no build artifacts, and (b) that made it easier to modify its requirements.

pip list | grep dash
- run:
name: 🧪 Run Integration Tests
command: |
. venv/bin/activate
TESTFILES=$(circleci tests glob "tests/integration/**/test_*.py" | circleci tests split --split-by=timings)
pytest --headless --nopercyfinalize --junitxml=test-reports/junit_intg.xml ${TESTFILES}
- store_artifacts:
Expand Down
4 changes: 4 additions & 0 deletions dash-renderer/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
- [#944](https://github.com/plotly/dash/pull/944) fixed bug with persistence being toggled on/off on an existing component

## [1.1.0] - 2019-09-17
### Added
- [#903](https://github.com/plotly/dash/pull/903) enables props edited by the user to persist across recreating the component or reloading the page. Components need to define three new props: `persistence`, `persisted_props`, and `persistence_type` as described in the lead comment of `src/persistence.js`. App developers then enable this behavior by, in the simplest case, setting `persistence: true` on the component. First use case is table, see [dash-table#566](https://github.com/plotly/dash-table/pull/566)
Expand Down
60 changes: 37 additions & 23 deletions dash-renderer/src/persistence.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,38 +280,41 @@ const getValsKey = (id, persistedProp, persistence) =>
`${id}.${persistedProp}.${JSON.stringify(persistence)}`;

const getProps = layout => {
const {props} = layout;
const {id, persistence} = props;
if (!id || !persistence) {
// This component doesn't have persistence. To make downstream
// tests more efficient don't return either one, so we just have to
// test for truthy persistence.
// But we still need to return props for consumers that look for
// nested components
const {props, type, namespace} = layout;
if (!type || !namespace) {
// not a real component - just need the props for recursion
return {props};
}
const {id, persistence} = props;

const element = Registry.resolve(layout);
const persisted_props =
props.persisted_props || element.defaultProps.persisted_props;
const persistence_type =
props.persistence_type || element.defaultProps.persistence_type;
if (!persisted_props || !persistence_type) {
return {props};
}
return {id, props, element, persistence, persisted_props, persistence_type};
const getVal = prop => props[prop] || (element.defaultProps || {})[prop];
const persisted_props = getVal('persisted_props');
const persistence_type = getVal('persistence_type');
const canPersist = id && persisted_props && persistence_type;

return {
canPersist,
id,
props,
element,
persistence,
persisted_props,
persistence_type,
};
};

export function recordUiEdit(layout, newProps, dispatch) {
const {
canPersist,
id,
props,
element,
persistence,
persisted_props,
persistence_type,
} = getProps(layout);
if (!persistence) {
if (!canPersist || !persistence) {
return;
}

Expand Down Expand Up @@ -378,6 +381,7 @@ function modProp(key, storage, element, props, persistedProp, update, undo) {

function persistenceMods(layout, component, path, dispatch) {
const {
canPersist,
id,
props,
element,
Expand All @@ -387,7 +391,7 @@ function persistenceMods(layout, component, path, dispatch) {
} = getProps(component);

let layoutOut = layout;
if (persistence) {
if (canPersist && persistence) {
const storage = getStore(persistence_type, dispatch);
const update = {};
forEach(
Expand Down Expand Up @@ -443,6 +447,7 @@ function persistenceMods(layout, component, path, dispatch) {
*/
export function prunePersistence(layout, newProps, dispatch) {
const {
canPersist,
id,
props,
persistence,
Expand All @@ -455,7 +460,7 @@ export function prunePersistence(layout, newProps, dispatch) {
propName in newProps ? newProps[propName] : prevVal;
const finalPersistence = getFinal('persistence', persistence);

if (!persistence && !finalPersistence) {
if (!canPersist || !(persistence || finalPersistence)) {
return newProps;
}

Expand All @@ -471,6 +476,8 @@ export function prunePersistence(layout, newProps, dispatch) {

const update = {};

let depersistedProps = props;

if (persistenceChanged && persistence) {
// clear previously-applied persistence
const storage = getStore(persistence_type, dispatch);
Expand All @@ -487,6 +494,7 @@ export function prunePersistence(layout, newProps, dispatch) {
),
filter(notInNewProps, persisted_props)
);
depersistedProps = mergeRight(props, update);
}

if (finalPersistence) {
Expand All @@ -497,10 +505,10 @@ export function prunePersistence(layout, newProps, dispatch) {
forEach(
persistedProp =>
modProp(
getValsKey(id, persistedProp, persistence),
getValsKey(id, persistedProp, finalPersistence),
finalStorage,
element,
props,
depersistedProps,
persistedProp,
update
),
Expand All @@ -516,11 +524,17 @@ export function prunePersistence(layout, newProps, dispatch) {
if (propTransforms) {
for (const propPart in propTransforms) {
finalStorage.removeItem(
getValsKey(id, `${propName}.${propPart}`, persistence)
getValsKey(
id,
`${propName}.${propPart}`,
finalPersistence
)
);
}
} else {
finalStorage.removeItem(getValsKey(id, propName, persistence));
finalStorage.removeItem(
getValsKey(id, propName, finalPersistence)
);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion dash/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
## Unreleased

### Added

- [#944](https://github.com/plotly/dash/pull/944)
- relevant `dash.testing` methods can now be called with either an element or a CSS selector: `select_dcc_dropdown`, `multiple_click`, `clear_input`, `zoom_in_graph_by_ratio`, `click_at_coord_fractions`.
- Three new `dash.testing` methods: `clear_local_storage`, `clear_session_storage`, and `clear_storage` (to clear both together)
- [#937](https://github.com/plotly/dash/pull/937) `dash.testing` adds two APIs `zoom_in_graph_by_ratio` and `click_at_coord_fractions` about advanced interactions using mouse `ActionChain`
- [#938](https://github.com/plotly/dash/issues/938) Adds debugging traces to dash backend about serving component suites, so we can use it to verify the installed packages whenever in doubt.

Expand Down
29 changes: 22 additions & 7 deletions dash/testing/browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ def find_elements(self, selector):
"""
return self.driver.find_elements_by_css_selector(selector)

def _get_element(self, elem_or_selector):
if isinstance(elem_or_selector, str):
return self.find_element(elem_or_selector)
return elem_or_selector

def _wait_for(self, method, args, timeout, msg):
"""abstract generic pattern for explicit WebDriverWait"""
_wait = (
Expand Down Expand Up @@ -262,8 +267,8 @@ def wait_for_page(self, url=None, timeout=10):
)
)

def select_dcc_dropdown(self, selector, value=None, index=None):
dropdown = self.driver.find_element_by_css_selector(selector)
def select_dcc_dropdown(self, elem_or_selector, value=None, index=None):
dropdown = self._get_element(elem_or_selector)
dropdown.click()

menu = dropdown.find_element_by_css_selector("div.Select-menu-outer")
Expand Down Expand Up @@ -416,13 +421,15 @@ def _get_firefox(self):
def _is_windows():
return sys.platform == "win32"

def multiple_click(self, selector, clicks):
def multiple_click(self, elem_or_selector, clicks):
"""multiple_click click the element with number of `clicks`"""
for _ in range(clicks):
self.find_element(selector).click()
self._get_element(elem_or_selector).click()

def clear_input(self, elem):
def clear_input(self, elem_or_selector):
"""simulate key press to clear the input"""
elem = self._get_element(elem_or_selector)

(
ActionChains(self.driver)
.click(elem)
Expand All @@ -434,12 +441,18 @@ def clear_input(self, elem):
).perform()

def zoom_in_graph_by_ratio(
self, elem, start_fraction=0.5, zoom_box_fraction=0.2, compare=True
self,
elem_or_selector,
start_fraction=0.5,
zoom_box_fraction=0.2,
compare=True
):
"""zoom out a graph with a zoom box fraction of component dimension
default start at middle with a rectangle of 1/5 of the dimension
use `compare` to control if we check the svg get changed
"""
elem = self._get_element(elem_or_selector)

prev = elem.get_attribute("innerHTML")
w, h = elem.size["width"], elem.size["height"]
try:
Expand All @@ -455,7 +468,9 @@ def zoom_in_graph_by_ratio(
"innerHTML"
), "SVG content should be different after zoom"

def click_at_coord_fractions(self, elem, fx, fy):
def click_at_coord_fractions(self, elem_or_selector, fx, fy):
elem = self._get_element(elem_or_selector)

ActionChains(self.driver).move_to_element_with_offset(
elem, elem.size["width"] * fx, elem.size["height"] * fy
).click().perform()
Expand Down
10 changes: 10 additions & 0 deletions dash/testing/dash_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,13 @@ def get_session_storage(self, session_id="session"):
session_id
)
)

def clear_local_storage(self):
self.driver.execute_script("window.localStorage.clear()")

def clear_session_storage(self):
self.driver.execute_script("window.sessionStorage.clear()")

def clear_storage(self):
self.clear_local_storage()
self.clear_session_storage()
Loading