In practice, `withActionsDisabled()` does not disable actions
While refactoring code, I started investigating what withActionsDisabled() claims to do, and what it actually does. And came to the conclusion that at least at the moment, it doesn't.
The core issue is that its operation is driven by this: https://github.com/mozilla/pontoon/blob/8d2714d5dbfd69e3188e620d02d28adcc06483a6/frontend/src/core/utils/components/withActionsDisabled.tsx#L30-L38
So when disableAction() is called:
- We get one render of the wrapped component with
isActionDisabled: true componentDidUpdate()is called due to the state change (also props change, via the redux store)isActionDisabledis setfalse- The wrapped component is rendered again, with
isActionDisabled: false
All of that happens too fast to affect human actions, which means that e.g. double clicking a status-change button sends the action twice. To show that to myself, I put together a proof of concept. Running that, and double clicking a reject button, I see this in the console:
wAD enable
{ action: "reject" }
wAD disable { sameProps: false, sameState: true }
wAD disable { sameProps: true, sameState: false }
wAD enable
{ action: "reject" }
wAD disable { sameProps: false, sameState: true }
wAD disable { sameProps: true, sameState: false }
I was also able to duplicate the same issue on the staging server by observing multiple REST calls being sent from a double-click.
We should do something about this, but I'm not really sure what?
- If this is working as intended, and it's disabling some other category of error cases, this should be clarified.
- If this isn't needed any more, it should be dropped.
- If double-clicking should only result in one action, and this should prevent that, it should be refactored/fixed to enable that.
Related: #2292