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

Added the header and background #46

Closed Nishant2907 closed 3 years ago

Nishant2907 commented 3 years ago

Kindly review the PR and tell if any changes are needed or not :)

Here's a screenshot of the header. The text in middle is moving in a typewriter format.

image

Nishant2907 commented 3 years ago

Solved issue #16 .

Raghwendra-Dey commented 3 years ago

@Nishant2907 nice work. But I have few issues:

Screenshot from 2020-12-08 23-58-53

Nishant2907 commented 3 years ago

Hey @Raghwendra-Dey , I have done some updates Kindly review them

Raghwendra-Dey commented 3 years ago

@Nishant2907 could you reduce the height of the navbar a bit?

Nishant2907 commented 3 years ago

Yes sure, any more changes that need to be done?

Raghwendra-Dey commented 3 years ago

Yes sure, any more changes that need to be done?

Nothing else. Please make the height which also looks good, and don't unncessarily hide the things below. I mean atleast you could see the whole difference block without scrolling.

Raghwendra-Dey commented 3 years ago

@Nishant2907 are you done with modifying the height?

Raghwendra-Dey commented 3 years ago

@Nishant2907 please resolve the merge conflicts.

Nishant2907 commented 3 years ago

@Raghwendra-Dey , i have solved the merge issues, you can merge the PR now

Raghwendra-Dey commented 3 years ago

@Nishant2907 few things are messed up, like now, theres two footers, the pop up messages images are not getting loaded, like reset button's pop up message is not getting loaded, also, -1 button's pop-up is also messed up. I think, you messed them, while resolving the conflicts, I suggest you revert the last commit, and redo it carefully. Make sure, you don't mess up something which is already implemented. I suggest you keep the master repo files opened side by side for easy reference.

Nishant2907 commented 3 years ago

Okay i am looking on it, but 1st thing I checked the file and found that getting two footers was not my fault. I got it from the fetch i did. So kindly check it once

Raghwendra-Dey commented 3 years ago

I cloned your patch, and it still shows two footers:

Screenshot from 2020-12-16 21-01-02

And since theres no footers in the master branch, so certainly those are introduced after your patch only. Once check that you are up to date with this PR, on your local machine. And also look into rest of the problems.

Nishant2907 commented 3 years ago

Kindly check now, i hope it should work now.

Raghwendra-Dey commented 3 years ago

@Nishant2907 the pop-up message thing is still not fixed, and the footer still have the How to use?? thing, which is supposed to go after your patch. Please correct em.

Nishant2907 commented 3 years ago

when i am checking, i am getting a popup on increasing or decreasing the amount of problems in the right corner of the screen. Can you show me how does the popup looks

Raghwendra-Dey commented 3 years ago

when i am checking, i am getting a popup on increasing or decreasing the amount of problems in the right corner of the screen. Can you show me how does the popup looks

check the hosted site, after your patch those are not same as its there.

Raghwendra-Dey commented 3 years ago

here is what they look like in your patch:

Screenshot from 2020-12-16 22-20-45 Screenshot from 2020-12-16 22-21-05

Nishant2907 commented 3 years ago

Kindly show, how it should look

Raghwendra-Dey commented 3 years ago

Kindly show, how it should look

you can see the hosted site.

Raghwendra-Dey commented 3 years ago

If you need any help with the pop-up part, you may contact @kri-bhawna , and take her help. She worked on this.

Nishant2907 commented 3 years ago

kindly check now, i hope it should work now

Raghwendra-Dey commented 3 years ago

kindly check now, i hope it should work now

Its still not fixed. One suggestion, always run your code yourself and ensure its correctness, before asking reviewers to see.

Nishant2907 commented 3 years ago

image its working for me.

Raghwendra-Dey commented 3 years ago

image its working for me.

+1 is working for me as well. -1 is not working, also the reset is not working.

Nishant2907 commented 3 years ago

Hey, i have checked the popup, its working now

Raghwendra-Dey commented 3 years ago

Sorry mate. Seems #64 broke the average time feature. I need to first get that fixed, then only I can merge this. Please hold untill that is done. Dont worry about the merge conflicts its showing now. Your patch is fine now, I will get #64 cleared then merge this.

Raghwendra-Dey commented 3 years ago

@Nishant2907 Thank you!