search

mathjazz / pontoon

In-place localization tool
https://pontoon.mozilla.org/
BSD 3-Clause "New" or "Revised" License
3 stars 1 forks source link

Address TypeScript issues for withActionsDisabled #955

Closed mathjazz closed 3 years ago

mathjazz commented 3 years ago

This issue was created automatically with bugzilla2github.

Bug 1710215

Bug Reporter: @Pike CC: adrian@gaudebert.fr, @mathjazz Blocker for: Bug 1685565

The migration to TS isn't happy with withActionsDisabled.js. I'd love for someone to take a look at that, mostly Adrian or Matjaz.

When Matjaz originally added that (Higher Order?) Component, it had a bunch of users, but now it's just src/modules/history/components/Translation.js. And the latter does use quite a bit of outer state, which I stared at cluelessly.

Maybe this needs a refactor?

mathjazz commented 3 years ago

Comment Author: Adrian Gaudebert <adrian@gaudebert.fr>

Yes, withActionsDisabled.js was added as a solution to the "button spamming" problem, but is actually not the correct solution. The correct solution is to have the associated action disable itself until the server has sent a response. I believe we have implemented that in some places, but not all of them. I would indeed recommend to refactor withActionsDisabled.js into non-existence. :-)

mathjazz commented 3 years ago

Comment Author: @Pike

I dared to dig into this, and when trying to find out which actions needed to be wrapped I hit https://github.com/mozilla/pontoon/blob/dda49774209fb4d98bddffa96d4fb28a88a679a5/frontend/src/modules/entitydetails/components/EntityDetails.js#L399-L403, which is a hook now elsewhere. As in, the method that comment refers to doesn't exist anymore. Or, I tried to punch above my weight, and then saw the light, and ran away scared.