pontoon icon indicating copy to clipboard operation
pontoon copied to clipboard

In practice, `withActionsDisabled()` does not disable actions

Open eemeli opened this issue 3 years ago • 1 comments

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:

  1. We get one render of the wrapped component with isActionDisabled: true
  2. componentDidUpdate() is called due to the state change (also props change, via the redux store)
  3. isActionDisabled is set false
  4. 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?

  1. If this is working as intended, and it's disabling some other category of error cases, this should be clarified.
  2. If this isn't needed any more, it should be dropped.
  3. If double-clicking should only result in one action, and this should prevent that, it should be refactored/fixed to enable that.

eemeli avatar Feb 26 '22 13:02 eemeli

Related: #2292

eemeli avatar Mar 04 '22 19:03 eemeli