kblincoe / VisualGit_SE701_2019_1

1 stars 0 forks source link

Fixes #60 - Stop panel appearing when selecting checkbox #146

Closed DionBalmforth closed 5 years ago

DionBalmforth commented 5 years ago

Related Issue: Closes #60

Description: Users can now tick the checkbox for files that they have added/ adjusted and they will not have the panel open. To open the panel the user must now click on a file that does not have its checkbox ticked, and to close the panel the user must select the same file that is currently open.

Testing: testing has not been set up yet, however, this functionality can be tested by:

emipeanz commented 5 years ago

Quick question, if you select a file that has been changed, the select another file that has been changed ie. open the panel of changes for file one, then click file2 to ideally open panel of changes, is it just meant to close again? I'm not sure if this is meant to be functionally like that or my machine has messed up again with pulling and installing things???

DionBalmforth commented 5 years ago

Quick question, if you select a file that has been changed, the select another file that has been changed ie. open the panel of changes for file one, then click file2 to ideally open panel of changes, is it just meant to close again? I'm not sure if this is meant to be functionally like that or my machine has messed up again with pulling and installing things???

No, that issue should have been fixed with this PR. You should be able to click a new file and it opens the panel immediately. Maybe try ticking a files checkbox, if doing that opens the panel then there was an issue when pulling the changes.

emipeanz commented 5 years ago

Yea, I can click a file and panel opens immediately, and clicking on the tick box doesn't open the panel. So I'm a bit confused lol

DionBalmforth commented 5 years ago

Yea, I can click a file and panel opens immediately, and clicking on the tick box doesn't open the panel. So I'm a bit confused lol

Oh ok, I believe that should have been fixed with this PR. If this problem still persists then it might pay to have a second pair of eyes look at this PR, maybe add a request in the slack channel?

aorthi commented 5 years ago

Quick question, if you select a file that has been changed, the select another file that has been changed ie. open the panel of changes for file one, then click file2 to ideally open panel of changes, is it just meant to close again? I'm not sure if this is meant to be functionally like that or my machine has messed up again with pulling and installing things???

Just confirming before I user test this - the expected response is that when I click on file 1 to open it, the panel of changes opens and then when I then click on file 2, it will immediately open its panel of changes instead? And the issue you're observing is when file 2 is clicked, it simply closes file 1's panel of changes and does not open the panel for file 2? @emipeanz @dbal234

aorthi commented 5 years ago

Just confirming before I user test this - the expected response is that when I click on file 1 to open it, the panel of changes opens and then when I then click on file 2, it will immediately open its panel of changes instead? And the issue you're observing is when file 2 is clicked, it simply closes file 1's panel of changes and does not open the panel for file 2? @emipeanz @dbal234

If this comment is correctly identifying the issue, then I can confirm I'm experiencing it too - single clicking file 2 only closes file 1's changes panel and an additional click is needed to open file 2's changes

emipeanz commented 5 years ago

I think that clicking the second is meant to open the panel straight up, @dbal234 will need to confirm. Thanks @aorthi , glad to know I didn't pull it incorrectly 😛

DionBalmforth commented 5 years ago

I think that clicking the second is meant to open the panel straight up, @dbal234 will need to confirm. Thanks @aorthi , glad to know I didn't pull it incorrectly 😛

Yes the second panel is meant to open straight up, I will check this on a different computer and then get back to you guys :)

aorthi commented 5 years ago

Huh, weirdly I pulled the PR again, ran npm install and then ran the application and cannot replicate the issue? Would be good to get another person trying it because I'm not too certain why this behaviour is happening

emipeanz commented 5 years ago

Ill try pulling again to see if I can replicate it .......

aorthi commented 5 years ago

Ill try pulling again to see if I can replicate it .......

@emipeanz maybe make sure you're on your master when you pull the PR and switch to it? I think I originally was on a branch and that shouldn't have caused issues, but that's the only thing that might've caused the pull to have messed up

DionBalmforth commented 5 years ago

Ill try pulling again to see if I can replicate it .......

@emipeanz maybe make sure you're on your master when you pull the PR and switch to it? I think I originally was on a branch and that shouldn't have caused issues, but that's the only thing that might've caused the pull to have messed up

Ok I have been able to replicate the issue on a different computer, I will try and figure out what is causing it

DionBalmforth commented 5 years ago

it seems that you need to run npm run-script createjs and it works fine :)

DionBalmforth commented 5 years ago

it seems that you need to run npm run-script createjs and it works fine :)

@emipeanz @aorthi

aorthi commented 5 years ago

it seems that you need to run npm run-script createjs and it works fine :)

Can confirm that this is what is working for me, the javascript has to be explicitly recompiled for it to work which seems a bit strange

bcox280 commented 5 years ago

In #133 I remove the ts compile that occurs in the post install script after npm install. This was because the compilation is now done in watch mode and remains watching when it is run (which is not what you want happening during npm install).

When you run npm start, createjs and the electon:serve script are run in parallel. I am unsure why you would have to explicitly recompile the ts for it to recognise the changes, as it always compiles instantly on start. My only guess as to why this is occurring is that the start script creates a race condition when it runs the two scripts in parallel. If the createjs script wins, then the electron reloader will not notice the compilation changes, but if the electron:serve script wins, then the reloader will notice the compilation changes.

Although that's just a theory. We might need to see if this keeps occurring in other PRs before making an issue.

emipeanz commented 5 years ago

I cannot get this working on my machine, even with a clear branch and pull - this is most likely something to do with my machine that I'm still trying to fix. @aorthi , did you want to sign off on this??