From 3c7cbeaecdcac351798aadc04aed7a144a1ed8c0 Mon Sep 17 00:00:00 2001 From: Simon Bos Date: Sat, 2 Nov 2019 23:35:36 +0100 Subject: [PATCH 1/6] Fix setState race conditions --- src/SnackbarItem/SnackbarItem.js | 13 +++ src/SnackbarProvider.js | 148 +++++++++++++++++++++---------- 2 files changed, 113 insertions(+), 48 deletions(-) diff --git a/src/SnackbarItem/SnackbarItem.js b/src/SnackbarItem/SnackbarItem.js index 4ea89130..2be275c1 100644 --- a/src/SnackbarItem/SnackbarItem.js +++ b/src/SnackbarItem/SnackbarItem.js @@ -28,6 +28,14 @@ class SnackbarItem extends Component { this.props.onClose(event, reason, key); }; + handleEntered = key => () => { + const { snack } = this.props; + if (snack.requestClose) + this.props.onClose(null, null, key) + else + this.props.onEntered(key); + } + handleExited = key => (event) => { const { onExited, snack: { onExited: singleOnExited } } = this.props; if (singleOnExited) singleOnExited(event, key); @@ -65,6 +73,8 @@ class SnackbarItem extends Component { action: singleAction, ContentProps: singleContentProps = {}, anchorOrigin, + requestClose, + entered, ...singleSnackProps } = snack; @@ -115,6 +125,7 @@ class SnackbarItem extends Component { open={snack.open} classes={getSnackbarClasses(classes)} onClose={this.handleClose(key)} + onEntered={this.handleEntered(key)} > {snackContent || ( { - this.queue = []; - } - /** * Adds a new snackbar to the queue to be presented. * @param {string} message - text of the notification @@ -49,14 +44,6 @@ class SnackbarProvider extends Component { * @returns generated or user defined key referencing the new snackbar or null */ enqueueSnackbar = (message, { key, preventDuplicate, ...options } = {}) => { - if (preventDuplicate || this.props.preventDuplicate) { - const inQueue = this.queue.findIndex(item => item.message === message) > -1; - const inView = this.state.snacks.findIndex(item => item.message === message) > -1; - if (inQueue || inView) { - return null; - } - } - const id = (key || key === 0) ? key : new Date().getTime() + Math.random(); const snack = { key: id, @@ -64,14 +51,27 @@ class SnackbarProvider extends Component { open: true, message, anchorOrigin: options.anchorOrigin || this.props.anchorOrigin, + requestClose: false, + entered: false, }; if (options.persist) { snack.autoHideDuration = undefined; } - this.queue.push(snack); - this.handleDisplaySnack(); + this.setState((state, props) => { + if (preventDuplicate || props.preventDuplicate) { + const inQueue = state.queue.findIndex(item => item.message === message) > -1; + const inView = state.snacks.findIndex(item => item.message === message) > -1; + if (inQueue || inView) { + return state; + } + } + return this.handleDisplaySnack({ + ...state, + queue: [...state.queue, snack] + }, props); + }); return id; }; @@ -79,71 +79,114 @@ class SnackbarProvider extends Component { /** * Display snack if there's space for it. Otherwise, immediately begin dismissing the * oldest message to start showing the new one. + * + * Note: this function is a reducer, i.e. it returns an updated state based on the + * current state and current props. + * @param {object} state - current state + * @param {object} props - current props */ - handleDisplaySnack = () => { - const { maxSnack } = this.props; - const { snacks } = this.state; + handleDisplaySnack = (state, props) => { + const { maxSnack } = props; + const { snacks } = state; if (snacks.length >= maxSnack) { - return this.handleDismissOldest(); + return this.handleDismissOldest(state, props); } - return this.processQueue(); + return this.processQueue(state, props); }; /** * Display items (notifications) in the queue if there's space for them. + * + * Note: this function is a reducer, i.e. it returns an updated state based on the + * current state and current props. + * @param {object} state - current state + * @param {object} props - current props */ - processQueue = () => { - if (this.queue.length > 0) { - const newOne = this.queue.shift(); - this.setState(({ snacks }) => ({ - snacks: [...snacks, newOne], - })); + processQueue = (state, props) => { + const { queue, snacks } = state; + if (queue.length > 0) { + return { + ...state, + snacks: [...snacks, queue[0]], + queue: queue.slice(1, queue.length), + } } + return state; }; /** * Hide oldest snackbar on the screen because there exists a new one which we have to display. * (ignoring the one with 'persist' flag. i.e. explicitly told by user not to get dismissed). + * + * Note 1: this function is a reducer, i.e. it returns an updated state based on the + * current state and current props. + * Note 2: If there is already a message leaving the screen, no new messages are dismissed. + * Note 3: If the oldest message has not yet entered the screen, only a request to close the + * snackbar is made. Once it entered the screen, it will be immediately dismissed. + * + * @param {object} state - current state + * @param {object} props - current props */ - handleDismissOldest = () => { + handleDismissOldest = (state, props) => { + if (state.snacks.some((item) => {return !item.open || item.requestClose})) { + return state; + } let popped = false; let ignore = false; - const persistentCount = this.state.snacks.reduce((acc, current) => ( + const persistentCount = state.snacks.reduce((acc, current) => ( acc + (current.open && current.persist ? 1 : 0) ), 0); - if (persistentCount === this.props.maxSnack) { + if (persistentCount === props.maxSnack) { warning(MESSAGES.NO_PERSIST_ALL); ignore = true; } - this.setState(({ snacks }) => ({ - snacks: snacks - .filter(item => item.open === true) + return { + ...state, + snacks: state.snacks .map((item) => { if (!popped && (!item.persist || ignore)) { popped = true; - if (item.onClose) item.onClose(null, 'maxsnack', item.key); - if (this.props.onClose) this.props.onClose(null, 'maxsnack', item.key); - return { - ...item, - open: false, - }; + if (!item.entered) { + return { + ...item, + requestClose: true, + }; + } else { + if (item.onClose) item.onClose(null, 'maxsnack', item.key); + if (props.onClose) props.onClose(null, 'maxsnack', item.key); + return { + ...item, + open: false + }; + } } - return { ...item, }; }), - })); + }; }; + /** + * Set the entered state of the snackbar with the given key. + * @param {number} key - id of the snackbar that entered. + */ + handleEnteredSnack = (key) => { + this.setState(({ snacks }) => ({ + snacks: snacks.map(item => ( + item.key === key ? { ...item, entered: true } : { ...item } + )), + })); + } + /** * Hide a snackbar after its timeout. * @param {object} event - The event source of the callback - * @param {string} reason - can be timeout or clickaway + * @param {string} reason - can be timeout, clickaway * @param {number} key - id of the snackbar we want to hide */ handleCloseSnack = (event, reason, key) => { @@ -153,10 +196,11 @@ class SnackbarProvider extends Component { if (reason === 'clickaway') return; - this.setState(({ snacks }) => ({ + this.setState(({ snacks, queue }) => ({ snacks: snacks.map(item => ( (!key || item.key === key) ? { ...item, open: false } : { ...item } )), + queue: queue.filter(item => item.key !== key), })); }; @@ -172,15 +216,22 @@ class SnackbarProvider extends Component { * When we set open attribute of a snackbar to false (i.e. after we hide a snackbar), * it leaves the screen and immediately after leaving animation is done, this method * gets called. We remove the hidden snackbar from state and then display notifications - * waiting in the queue (if any). + * waiting in the queue (if any). If after this process the queue is not empty, the + * oldest message is dismissed. * @param {number} key - id of the snackbar we want to remove * @param {object} event - The event source of the callback */ handleExitedSnack = (event, key) => { - this.setState(({ snacks }) => ({ - snacks: snacks.filter(item => item.key !== key), - }), this.handleDisplaySnack); - + this.setState((state, props) => { + const newState = this.processQueue({ + ...state, + snacks: state.snacks.filter(item => item.key !== key), + }, props); + if (newState.queue.length == 0) + return newState; + else + return this.handleDismissOldest(newState, props); + }); if (this.props.onExited) this.props.onExited(event, key); }; @@ -219,6 +270,7 @@ class SnackbarProvider extends Component { classes={getClasses(classes)} onClose={this.handleCloseSnack} onExited={this.handleExitedSnack} + onEntered={this.handleEnteredSnack} /> ))} From 19b42bcd166939bfd823c0009f1cf9a4796a84c8 Mon Sep 17 00:00:00 2001 From: Hossein Dehnokhalaji Date: Tue, 5 Nov 2019 00:44:13 +0000 Subject: [PATCH 2/6] Call user-defined onEntered callbacks. Remove props from reducers. Minor refactoring. --- src/SnackbarItem/SnackbarItem.js | 16 ++-- src/SnackbarProvider.js | 133 ++++++++++++++----------------- 2 files changed, 72 insertions(+), 77 deletions(-) diff --git a/src/SnackbarItem/SnackbarItem.js b/src/SnackbarItem/SnackbarItem.js index 2be275c1..7ba201b8 100644 --- a/src/SnackbarItem/SnackbarItem.js +++ b/src/SnackbarItem/SnackbarItem.js @@ -7,7 +7,7 @@ import Collapse from '@material-ui/core/Collapse'; import SnackbarContent from '@material-ui/core/SnackbarContent'; import { getTransitionDirection, getSnackbarClasses, getCollapseClasses } from './SnackbarItem.util'; import styles from './SnackbarItem.styles'; -import { capitalise, MESSAGES } from '../utils/constants'; +import { capitalise, MESSAGES, REASONS } from '../utils/constants'; import warning from '../utils/warning'; @@ -28,12 +28,16 @@ class SnackbarItem extends Component { this.props.onClose(event, reason, key); }; - handleEntered = key => () => { + handleEntered = key => (node, isAppearing) => { const { snack } = this.props; - if (snack.requestClose) - this.props.onClose(null, null, key) - else - this.props.onEntered(key); + if (snack.requestClose) { + this.handleClose(key)(null, REASONS.MAXSNACK); + } else { + if (snack.onEntered) { + snack.onEntered(node, isAppearing, key); + } + this.props.onEntered(node, isAppearing, key); + } } handleExited = key => (event) => { diff --git a/src/SnackbarProvider.js b/src/SnackbarProvider.js index 308ae06c..32c81cce 100644 --- a/src/SnackbarProvider.js +++ b/src/SnackbarProvider.js @@ -2,7 +2,7 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import Slide from '@material-ui/core/Slide'; import SnackbarContext from './SnackbarContext'; -import { MESSAGES, defaultIconVariant, originKeyExtractor, allClasses } from './utils/constants'; +import { MESSAGES, defaultIconVariant, originKeyExtractor, allClasses, REASONS } from './utils/constants'; import SnackbarItem from './SnackbarItem'; import SnackbarContainer from './SnackbarContainer'; import warning from './utils/warning'; @@ -44,27 +44,28 @@ class SnackbarProvider extends Component { * @returns generated or user defined key referencing the new snackbar or null */ enqueueSnackbar = (message, { key, preventDuplicate, ...options } = {}) => { - const id = (key || key === 0) ? key : new Date().getTime() + Math.random(); + const userSpecifiedKey = key || key === 0; + const id = userSpecifiedKey ? key : new Date().getTime() + Math.random(); const snack = { key: id, ...options, - open: true, message, - anchorOrigin: options.anchorOrigin || this.props.anchorOrigin, - requestClose: false, + open: true, entered: false, + requestClose: false, + anchorOrigin: options.anchorOrigin || this.props.anchorOrigin, }; if (options.persist) { snack.autoHideDuration = undefined; } - this.setState((state, props) => { + this.setState((state) => { if ((preventDuplicate === undefined && this.props.preventDuplicate) || preventDuplicate) { const compareFunction = item => ( - (key || key === 0) ? item.key === key : item.message === message + userSpecifiedKey ? item.key === key : item.message === message ); - + const inQueue = this.queue.findIndex(compareFunction) > -1; const inView = this.state.snacks.findIndex(compareFunction) > -1; if (inQueue || inView) { @@ -75,39 +76,28 @@ class SnackbarProvider extends Component { return this.handleDisplaySnack({ ...state, queue: [...state.queue, snack] - }, props); + }); }); return id; }; /** - * Display snack if there's space for it. Otherwise, immediately begin dismissing the - * oldest message to start showing the new one. - * - * Note: this function is a reducer, i.e. it returns an updated state based on the - * current state and current props. - * @param {object} state - current state - * @param {object} props - current props + * Reducer: Display snack if there's space for it. Otherwise, immediately + * begin dismissing the oldest message to start showing the new one. */ - handleDisplaySnack = (state, props) => { - const { maxSnack } = props; + handleDisplaySnack = (state) => { const { snacks } = state; - if (snacks.length >= maxSnack) { - return this.handleDismissOldest(state, props); + if (snacks.length >= this.props.maxSnack) { + return this.handleDismissOldest(state); } - return this.processQueue(state, props); + return this.processQueue(state); }; /** - * Display items (notifications) in the queue if there's space for them. - * - * Note: this function is a reducer, i.e. it returns an updated state based on the - * current state and current props. - * @param {object} state - current state - * @param {object} props - current props + * Reducer: Display items (notifications) in the queue if there's space for them. */ - processQueue = (state, props) => { + processQueue = (state) => { const { queue, snacks } = state; if (queue.length > 0) { return { @@ -120,22 +110,18 @@ class SnackbarProvider extends Component { }; /** - * Hide oldest snackbar on the screen because there exists a new one which we have to display. + * Reducer: Hide oldest snackbar on the screen because there exists a new one which we have to display. * (ignoring the one with 'persist' flag. i.e. explicitly told by user not to get dismissed). * - * Note 1: this function is a reducer, i.e. it returns an updated state based on the - * current state and current props. - * Note 2: If there is already a message leaving the screen, no new messages are dismissed. - * Note 3: If the oldest message has not yet entered the screen, only a request to close the + * Note 1: If there is already a message leaving the screen, no new messages are dismissed. + * Note 2: If the oldest message has not yet entered the screen, only a request to close the * snackbar is made. Once it entered the screen, it will be immediately dismissed. - * - * @param {object} state - current state - * @param {object} props - current props */ - handleDismissOldest = (state, props) => { - if (state.snacks.some((item) => {return !item.open || item.requestClose})) { + handleDismissOldest = (state) => { + if (state.snacks.some(item => !item.open || item.requestClose)) { return state; } + let popped = false; let ignore = false; @@ -143,44 +129,44 @@ class SnackbarProvider extends Component { acc + (current.open && current.persist ? 1 : 0) ), 0); - if (persistentCount === props.maxSnack) { + if (persistentCount === this.props.maxSnack) { warning(MESSAGES.NO_PERSIST_ALL); ignore = true; } - return { - ...state, - snacks: state.snacks - .map((item) => { - if (!popped && (!item.persist || ignore)) { - popped = true; - - if (!item.entered) { - return { - ...item, - requestClose: true, - }; - } else { - if (item.onClose) item.onClose(null, 'maxsnack', item.key); - if (props.onClose) props.onClose(null, 'maxsnack', item.key); - return { - ...item, - open: false - }; - } - } + const snacks = state.snacks.map((item) => { + if (!popped && (!item.persist || ignore)) { + popped = true; + + if (!item.entered) { return { ...item, + requestClose: true, }; - }), - }; + } else { + if (item.onClose) item.onClose(null, REASONS.MAXSNACK, item.key); + if (this.props.onClose) this.props.onClose(null, REASONS.MAXSNACK, item.key); + return { + ...item, + open: false, + }; + } + } + + return { ...item }; + }); + + return { ...state, snacks }; }; /** * Set the entered state of the snackbar with the given key. - * @param {number} key - id of the snackbar that entered. */ - handleEnteredSnack = (key) => { + handleEnteredSnack = (node, isAppearing, key) => { + if (this.props.onEntered) { + this.props.onEntered(node, isAppearing, key); + } + this.setState(({ snacks }) => ({ snacks: snacks.map(item => ( item.key === key ? { ...item, entered: true } : { ...item } @@ -199,7 +185,7 @@ class SnackbarProvider extends Component { this.props.onClose(event, reason, key); } - if (reason === 'clickaway') return; + if (reason === REASONS.CLICKAWAY) return; this.setState(({ snacks, queue }) => ({ snacks: snacks.map(item => ( @@ -227,17 +213,22 @@ class SnackbarProvider extends Component { * @param {object} event - The event source of the callback */ handleExitedSnack = (event, key) => { - this.setState((state, props) => { + this.setState((state) => { const newState = this.processQueue({ ...state, snacks: state.snacks.filter(item => item.key !== key), - }, props); - if (newState.queue.length == 0) + }); + + if (newState.queue.length === 0) { return newState; - else - return this.handleDismissOldest(newState, props); + } + + return this.handleDismissOldest(newState); }); - if (this.props.onExited) this.props.onExited(event, key); + + if (this.props.onExited) { + this.props.onExited(event, key); + } }; render() { From ce332711b4ca7bb88ddf24791ed4b9f2e0e456b0 Mon Sep 17 00:00:00 2001 From: Hossein Dehnokhalaji Date: Tue, 5 Nov 2019 00:44:44 +0000 Subject: [PATCH 3/6] leftover --- src/utils/constants.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/utils/constants.js b/src/utils/constants.js index 813b472b..2237a413 100644 --- a/src/utils/constants.js +++ b/src/utils/constants.js @@ -81,3 +81,8 @@ export const SNACKBAR_INDENTS = { export const capitalise = text => text.charAt(0).toUpperCase() + text.slice(1); export const originKeyExtractor = anchor => `${capitalise(anchor.vertical)}${capitalise(anchor.horizontal)}`; + +export const REASONS = { + CLICKAWAY: 'clickaway', + MAXSNACK: 'maxsnack', +}; From 3c31c4cb12230ee0c3097c6acee659172fa38eea Mon Sep 17 00:00:00 2001 From: Simon Bos Date: Tue, 5 Nov 2019 22:34:30 +0100 Subject: [PATCH 4/6] fix_regressions --- src/SnackbarProvider.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SnackbarProvider.js b/src/SnackbarProvider.js index 32c81cce..822e57be 100644 --- a/src/SnackbarProvider.js +++ b/src/SnackbarProvider.js @@ -66,10 +66,10 @@ class SnackbarProvider extends Component { userSpecifiedKey ? item.key === key : item.message === message ); - const inQueue = this.queue.findIndex(compareFunction) > -1; - const inView = this.state.snacks.findIndex(compareFunction) > -1; + const inQueue = state.queue.findIndex(compareFunction) > -1; + const inView = state.snacks.findIndex(compareFunction) > -1; if (inQueue || inView) { - return null; + return state; } } From 8697db7c821dccc1ad70a543a0725901d2f8b705 Mon Sep 17 00:00:00 2001 From: Simon Bos Date: Fri, 8 Nov 2019 01:44:48 +0100 Subject: [PATCH 5/6] handleCloseSnackbar update with entered state --- src/SnackbarItem/SnackbarItem.js | 9 ++++----- src/SnackbarProvider.js | 9 +++++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/SnackbarItem/SnackbarItem.js b/src/SnackbarItem/SnackbarItem.js index 7ba201b8..b092a7a1 100644 --- a/src/SnackbarItem/SnackbarItem.js +++ b/src/SnackbarItem/SnackbarItem.js @@ -30,13 +30,12 @@ class SnackbarItem extends Component { handleEntered = key => (node, isAppearing) => { const { snack } = this.props; + if (snack.onEntered) { + snack.onEntered(node, isAppearing, key); + } + this.props.onEntered(node, isAppearing, key); if (snack.requestClose) { this.handleClose(key)(null, REASONS.MAXSNACK); - } else { - if (snack.onEntered) { - snack.onEntered(node, isAppearing, key); - } - this.props.onEntered(node, isAppearing, key); } } diff --git a/src/SnackbarProvider.js b/src/SnackbarProvider.js index 822e57be..91b56b57 100644 --- a/src/SnackbarProvider.js +++ b/src/SnackbarProvider.js @@ -186,12 +186,17 @@ class SnackbarProvider extends Component { } if (reason === REASONS.CLICKAWAY) return; + const shouldRemoveAll = key === undefined; this.setState(({ snacks, queue }) => ({ snacks: snacks.map(item => ( - (!key || item.key === key) ? { ...item, open: false } : { ...item } + shouldRemoveAll || item.key === key + ? item.entered + ? { ...item, open: false } + : { ...item, requestClose: true } + : { ...item } )), - queue: queue.filter(item => item.key !== key), + queue: queue.filter(item => item.key !== key), // eslint-disable-line react/no-unused-state })); }; From df89289d5a5ce76841eafae8c5fde1d41102cb05 Mon Sep 17 00:00:00 2001 From: Hossein Dehnokhalaji Date: Fri, 8 Nov 2019 19:13:58 +0000 Subject: [PATCH 6/6] Minor linting fixes --- src/SnackbarItem/SnackbarItem.js | 2 ++ src/SnackbarProvider.js | 39 +++++++++++++++++--------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/SnackbarItem/SnackbarItem.js b/src/SnackbarItem/SnackbarItem.js index b092a7a1..a50d5200 100644 --- a/src/SnackbarItem/SnackbarItem.js +++ b/src/SnackbarItem/SnackbarItem.js @@ -34,6 +34,7 @@ class SnackbarItem extends Component { snack.onEntered(node, isAppearing, key); } this.props.onEntered(node, isAppearing, key); + if (snack.requestClose) { this.handleClose(key)(null, REASONS.MAXSNACK); } @@ -184,6 +185,7 @@ SnackbarItem.propTypes = { dense: PropTypes.bool.isRequired, onClose: PropTypes.func.isRequired, onExited: PropTypes.func.isRequired, + onEntered: PropTypes.func.isRequired, }; export default withStyles(styles)(SnackbarItem); diff --git a/src/SnackbarProvider.js b/src/SnackbarProvider.js index 91b56b57..8d16e07b 100644 --- a/src/SnackbarProvider.js +++ b/src/SnackbarProvider.js @@ -23,7 +23,7 @@ class SnackbarProvider extends Component { super(props); this.state = { snacks: [], - queue: [], + queue: [], // eslint-disable-line react/no-unused-state contextValue: { enqueueSnackbar: this.enqueueSnackbar, closeSnackbar: this.closeSnackbar, @@ -75,7 +75,7 @@ class SnackbarProvider extends Component { return this.handleDisplaySnack({ ...state, - queue: [...state.queue, snack] + queue: [...state.queue, snack], }); }); @@ -83,7 +83,7 @@ class SnackbarProvider extends Component { }; /** - * Reducer: Display snack if there's space for it. Otherwise, immediately + * Reducer: Display snack if there's space for it. Otherwise, immediately * begin dismissing the oldest message to start showing the new one. */ handleDisplaySnack = (state) => { @@ -104,7 +104,7 @@ class SnackbarProvider extends Component { ...state, snacks: [...snacks, queue[0]], queue: queue.slice(1, queue.length), - } + }; } return state; }; @@ -143,14 +143,15 @@ class SnackbarProvider extends Component { ...item, requestClose: true, }; - } else { - if (item.onClose) item.onClose(null, REASONS.MAXSNACK, item.key); - if (this.props.onClose) this.props.onClose(null, REASONS.MAXSNACK, item.key); - return { - ...item, - open: false, - }; } + + if (item.onClose) item.onClose(null, REASONS.MAXSNACK, item.key); + if (this.props.onClose) this.props.onClose(null, REASONS.MAXSNACK, item.key); + + return { + ...item, + open: false, + }; } return { ...item }; @@ -186,16 +187,18 @@ class SnackbarProvider extends Component { } if (reason === REASONS.CLICKAWAY) return; - const shouldRemoveAll = key === undefined; + const shouldCloseAll = key === undefined; this.setState(({ snacks, queue }) => ({ - snacks: snacks.map(item => ( - shouldRemoveAll || item.key === key - ? item.entered + snacks: snacks.map((item) => { + if (!shouldCloseAll && item.key !== key) { + return { ...item }; + } + + return item.entered ? { ...item, open: false } - : { ...item, requestClose: true } - : { ...item } - )), + : { ...item, requestClose: true }; + }), queue: queue.filter(item => item.key !== key), // eslint-disable-line react/no-unused-state })); };