Community
Participate
Working Groups
In gitStatusExplorer.js and gitCommitExplorer.js, there is a timeout before the compare widget or stage/unstage buttons are rendered. [This is an extraction from one of my patches in Bug 408862]
Sped up the rendering of the stage/unstage and edit file sprite buttons in git status, and rendering of the compare widget in git status and git commit explorer (/git/git-commit.html#) https://github.com/gabrielluong/orion.client/commit/ce79ccb349b4a2e912ca995451eda5d44bf73c16 https://orion.eclipse.org/git/reviewRequest.html#https://github.com/gabrielluong/orion.client.git_ce79ccb349b4a2e912ca995451eda5d44bf73c16 I assert that I authored 100% of the content of this contribution and have the rights to donate the content to Eclipse under the EPL
Most of the patch is OK, however I have doubts about getting rid of setTimeout functions. They seem to be added to asynchronously generate some more complex UI parts. In the original version when user clicks to open the compare editor the twistie is switched straight away together with adding the div for the compare editor and then the compare widget is constructed (because setTimeout function is called later). After your change constructing the original widget is run as blocking, so we first construct the compare widget and then open the twistie. If constructing the compare widget takes a while user will see a delay between clicking the twistie and any UI change, it may seem at first like the click did not work. While I agree that 300 or 500 millis is too much for setTimeout, maybe it would be better just to set the delay for 1?
Reduced delay in async rendering of the compare widget and stage/unstage buttons in git status and commit explorer https://github.com/gabrielluong/orion.client/commit/a81831026856649027d6beb9d3a530a8ba2c9de3 https://orion.eclipse.org/git/reviewRequest.html#https://github.com/gabrielluong/orion.client.git_a81831026856649027d6beb9d3a530a8ba2c9de3 I assert that I authored 100% of the content of this contribution and have the rights to donate the content to Eclipse under the EPL
At least from an architectural POV I thought the original fix made sense. As a general rule we should not be using setTimeout in our UI. If a widget takes a long time to render that would indicate a problem in the rendering code and so even if we use setTimeout we're just delaying the inevitable. There are of course exceptions where we genuinely do want asynch rendering however they should be very rare (and even then I'm skeptical) and in those cases we should be using a timeout of 0. (Also -- prefer using [].forEach instead of a for loop.)
I can see that everywhere in our code createCompareWidget is used inside of setTimeout, so I assumed that it might be heavy and we want to give the response after click as soon as possible. When having normal workload on my machine I don't see a delay when opening the twistie, because downloading and rendering the diff is asynchronous anyway. Maybe it wasn't always like this and this is a reason setTimeout was used in the first place. I preferred to be on the safe side, because it's RC2, but if Simon is OK, then I can commit the first version of the fix.
I did some more testing and I did not notice the delay in opening the twistie, so I assume that whatever caused using setTimeout is not present any more. I will then commit the first version of the fix, completely removing the setTimeout.
The first version of the patch merged.
Thanks Malgorzata!
*** Bug 379510 has been marked as a duplicate of this bug. ***
