Skip to content
This repository has been archived by the owner on Jul 6, 2020. It is now read-only.

Leaderboard decimal precision #314

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

Kajol-Kumari
Copy link
Member

@Kajol-Kumari Kajol-Kumari commented Apr 3, 2020

Fixes: #284 [ Add option to change leader-board decimal precision ]

  • Leaderboard precision value selector slider added under Leaderboard tab

  • Mat-slider has the default value of 2 on load.

  • Selected value gets updated in the backend

GIF:
ezgif com-video-to-gif

GIF showing default value and css for different screen sizes:
ezgif com-video-to-gif

@codecov-io
Copy link

codecov-io commented Apr 3, 2020

Codecov Report

Merging #314 into master will decrease coverage by 0.09%.
The diff coverage is 15.38%.

@@            Coverage Diff             @@
##           master     #314      +/-   ##
==========================================
- Coverage   50.62%   50.52%   -0.10%     
==========================================
  Files          66       66              
  Lines        3771     3784      +13     
  Branches      444      444              
==========================================
+ Hits         1909     1912       +3     
- Misses       1767     1777      +10     
  Partials       95       95              
Impacted Files Coverage Δ
...lengeleaderboard/challengeleaderboard.component.ts 32.81% <15.38%> (-1.27%) ⬇️
...bliclists/challengelist/challengelist.component.ts 47.57% <0.00%> (+0.97%) ⬆️
Impacted Files Coverage Δ
...lengeleaderboard/challengeleaderboard.component.ts 32.81% <15.38%> (-1.27%) ⬇️
...bliclists/challengelist/challengelist.component.ts 47.57% <0.00%> (+0.97%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3d23ca...c70b0a8. Read the comment docs.

@Kajol-Kumari Kajol-Kumari marked this pull request as ready for review April 4, 2020 16:11
@Kajol-Kumari
Copy link
Member Author

Kajol-Kumari commented Apr 4, 2020

@Sanji515 @Shekharrajak @RishabhJain2018 please take a look at it and if any changes are required, please let me know.

src/app/app.module.ts Outdated Show resolved Hide resolved
@RishabhJain2018
Copy link
Member

@Sanji515 Can you please review this again?

@deshraj deshraj requested a review from Sanji515 May 20, 2020 09:00
@codecov-commenter
Copy link

codecov-commenter commented May 20, 2020

Codecov Report

Merging #314 into master will decrease coverage by 0.10%.
The diff coverage is 21.42%.

@@            Coverage Diff             @@
##           master     #314      +/-   ##
==========================================
- Coverage   50.16%   50.06%   -0.11%     
==========================================
  Files          66       66              
  Lines        3873     3887      +14     
  Branches      450      450              
==========================================
+ Hits         1943     1946       +3     
- Misses       1836     1847      +11     
  Partials       94       94              
Impacted Files Coverage Δ
...lengeleaderboard/challengeleaderboard.component.ts 32.11% <21.42%> (-0.74%) ⬇️
Impacted Files Coverage Δ
...lengeleaderboard/challengeleaderboard.component.ts 32.11% <21.42%> (-0.74%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 932a9c0...3ebe757. Read the comment docs.

@Kajol-Kumari Kajol-Kumari force-pushed the leaderboard_decimal_precision branch from 328595c to 043f5a5 Compare May 20, 2020 17:01
@Sanji515
Copy link
Member

@Kajol-Kumari After updating the decimal precision value using the slider the leaderboard gets refreshed but still the score metric is not updating. Like if I the score is 20 and if I change the leaderboard decimal precision value to 4 then it should display as 20.0000

@Sanji515
Copy link
Member

@Kajol-Kumari One more change 😃

The circle with the decimal precision value looks a bit large, can we make it small

@Kajol-Kumari
Copy link
Member Author

@Kajol-Kumari One more change

The circle with the decimal precision value looks a bit large, can we make it small

Screenshot from 2020-05-21 00-48-54

@Sanji515
Copy link
Member

@Kajol-Kumari Please resolve the conflicts here.

@Sanji515
Copy link
Member

@Kajol-Kumari On smaller screens I don't see that circle showing leaderboard decimal precision value. Please look into it.
And also can we reduce the space between the slider and circle.

@Kajol-Kumari
Copy link
Member Author

@Kajol-Kumari On smaller screens I don't see that circle showing leaderboard decimal precision value. Please look into it.
And also can we reduce the space between the slider and circle.

@Sanji515 I had made this value hide for small and medium screen as the slider itself also shows the value. But now i changed it so that the value will appear on all screen sizes. Please check this once.

Copy link
Member

@Shekharrajak Shekharrajak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for wider screen. Not sure about tablet view. I see px in css

Copy link
Member

@Sanji515 Sanji515 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM This can be merged now 🎉

Side note: I think we can change the placing of slider in another PR to somewhere else for better look and to save some spacing.

@Kajol-Kumari Kajol-Kumari force-pushed the leaderboard_decimal_precision branch from 224c7e9 to f7a3f7d Compare May 30, 2020 23:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to change leader-board decimal precision
6 participants