Pop up message - Githubissues

Raghwendra-Dey / productivity_meter

Productivity meter for measuring productivity of solving problems

https://raghwendra-dey.github.io/productivity_meter/

MIT License

13 stars 23 forks source link

Pop up message #64

Closed kri-bhawna closed 3 years ago

kri-bhawna commented 3 years ago

fixes #21

kri-bhawna commented 3 years ago

@Raghwendra-Dey Sir, please give your feedback regarding the PR.

Raghwendra-Dey commented 3 years ago

Hey @kri-bhawna great work!! I have few points:

could you make the pop-ups a bit more towards the bottom right. The Lets begin pop up seems to be well placed, but other i think needs to be a bit more cornered.
could you make the Lets begin, and You are already at problem zero pop ups also have that fading effects?
And if you press the +1 or -1 buttons continously without stop then there seems to be some problem, since after sometime, the fading becomes too quick. Press the +1 button continuosly untill the fading starts, now you can see that the hold on time have become very small. Could you check this out, I think the fading time, maybe getting added up, or some other problem.

kri-bhawna commented 3 years ago

@Raghwendra-Dey I have made all the changes mentioned above . Please give you feedback regarding this. Thanking you.

Raghwendra-Dey commented 3 years ago

Nice work @kri-bhawna could you briefly explain what was the problem with the third point before, and how you fixed it?

Raghwendra-Dey commented 3 years ago

@kri-bhawna you broke the average time feature. Please look, and correct it. And also debug statement thing. I am reverting your PR for the time being.

kri-bhawna commented 3 years ago

@Raghwendra-Dey Regarding https://github.com/Raghwendra-Dey/productivity_meter/pull/64#issuecomment-746165965 . The timeout function was not responding properly when say +1 was clicked before its opacity reached 0. And the next click's effect was taking place in the left out remaining time of the first fade out effect . For example, when +1 is clicked for the first time , fade out effect starts to take place after say 3 sec and the whole transition will take place in say 2 sec. If one clicks +1 again after 4 sec have passed since the first click , the second transition was taking place in the remaining 1 sec. For this I had to introduce clear timeout and had to use clear interval for clearing the previous effects .

Raghwendra-Dey commented 3 years ago

@kri-bhawna After your patch the average time/problem thing isn't working properly. Thats why I needed to revert this PR.

kri-bhawna commented 3 years ago

@Raghwendra-Dey I am sorry regarding that , I just checked my part of work before submitting and was unaware of this. I will look into the issue. Sorry again.

kri-bhawna commented 3 years ago

@Raghwendra-Dey Just give me sometime , I am looking into this.

Raghwendra-Dey commented 3 years ago

@Raghwendra-Dey I am sorry regarding that , I just checked my part of work before submitting and was unaware of this. I will look into the issue. Sorry again.

Thats fine. This happens. Take your time. One suggestion before submitting any PR thouroughly verify every features. Its also my bad. I also missed that thing.

kri-bhawna commented 3 years ago

@Raghwendra-Dey Has there been any discussions regarding this. If yes please tell me. It may help in debugging.

Raghwendra-Dey commented 3 years ago

@Raghwendra-Dey Has there been any discussions regarding this. If yes please tell me. It may help in debugging.

None. I think you messed up something in the changes in prb_plus function. since you haven't touched any other function used by this feature. You can open the master repo and your code side by side, and look for the bug.

kri-bhawna commented 3 years ago

@Raghwendra-Dey I found the bug and have fixed it . The average time is working well. But I am unable to edit the code. Do I need to create a new PR ? Also, while creating a PR , I am getting a caution that it cannot be merged automatically.

Raghwendra-Dey commented 3 years ago

create a new PR.