Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show number of days left for long lasting contests. #1094

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

Conversation

eljakim
Copy link
Contributor

@eljakim eljakim commented Jan 7, 2019

When a contest lasts for a couple of weeks, showing the exact number of seconds counting down is strange and distracting. This change keeps everything as it was if there are less than 72 hours left in a contest. However, when there is more time left it only displays the number of days left.


This change is Reviewable

…king clock, when more than 72 hours are left in the contest.
@codecov
Copy link

codecov bot commented Jan 7, 2019

Codecov Report

Merging #1094 into master will decrease coverage by 0.21%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1094      +/-   ##
==========================================
- Coverage    62.2%   61.98%   -0.22%     
==========================================
  Files         229      230       +1     
  Lines       16520    16585      +65     
==========================================
+ Hits        10276    10280       +4     
- Misses       6244     6305      +61
Flag Coverage Δ
#functionaltests 45.78% <ø> (-0.28%) ⬇️
#unittests 43.28% <ø> (-0.04%) ⬇️
Impacted Files Coverage Δ
cms/server/contest/handlers/main.py 42.48% <0%> (-12.86%) ⬇️
cms/server/contest/handlers/tasksubmission.py 43.1% <0%> (-3.45%) ⬇️
cms/db/base.py 85.1% <0%> (-3.2%) ⬇️
cms/grading/Job.py 86.25% <0%> (-2.85%) ⬇️
cms/server/contest/handlers/contest.py 85.85% <0%> (-2.03%) ⬇️
cms/service/ProxyService.py 57.44% <0%> (-1.6%) ⬇️
cms/db/util.py 52.59% <0%> (-0.75%) ⬇️
cms/io/rpc.py 91.8% <0%> (-0.69%) ⬇️
cms/db/filecacher.py 77.04% <0%> (-0.66%) ⬇️
cms/server/contest/handlers/taskusertest.py 43.03% <0%> (-0.61%) ⬇️
... and 15 more

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 9335527...c2b9741. Read the comment docs.

@karliss
Copy link
Contributor

karliss commented Jan 7, 2019

Not everyone speaks uses English language. "days" should use the same translation mechanism as everything else.

@wil93
Copy link
Member

wil93 commented Jan 7, 2019

Maybe a good solution could be to show DD:HH:MM:SS instead of HH:MM:SS

No translation mechanism needed, would still retain the "precise" feeling, while at the same time solving the problem of "72 hours left"...

OR... we could just use a library to handle both the "humanization" of the time and its localization (e.g. moment or luxon)

@eljakim
Copy link
Contributor Author

eljakim commented Jan 7, 2019

Maybe a good solution could be to show DD:HH:MM:SS instead of HH:MM:SS

No translation mechanism needed, would still retain the "precise" feeling, while at the same time solving the problem of "72 hours left"...

The issue I have with the current system (besides the many hours that are left for a contest that lasts for two weeks) is that the ticking hours, minutes and seconds distract from the fact that they have plenty of time left.

I can definitely live with using moment.js or something similar, but even then you'd have to figure out what you want to show when. The way the system works is great for short term contests, but a bit over precise for longer term things.

Copy link
Member

@stefano-maggiolo stefano-maggiolo left a comment

Choose a reason for hiding this comment

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

I agree the seconds ticking might be unnerving, especially, but not only, when there are several days left :)

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @eljakim)


cms/server/contest/static/cws_utils.js, line 219 at r2 (raw file):

    if (days <= 2) {
	// if less than 72 hours left, return the exact number of hours.

Proper indentation and capitalization please.


cms/server/contest/static/cws_utils.js, line 225 at r2 (raw file):

    }
    else {
	// otherwise only return the number of days.

Ditto


cms/server/contest/templates/contest.html, line 153 at r2 (raw file):

        </div>
        <div style="display: none" id="translation_days">
            {% trans %}days{% endtrans %}

I think you need to translate %d days as some language might invert the order... I'm not sure there is a better way to do it in JS, but then I'd just to a replace("%d", days). @lerks for his extensive localization knowledge.

Copy link
Contributor Author

@eljakim eljakim left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @stefano-maggiolo)


cms/server/contest/static/cws_utils.js, line 219 at r2 (raw file):

Previously, stefano-maggiolo (Stefano Maggiolo) wrote…

Proper indentation and capitalization please.

Done.


cms/server/contest/static/cws_utils.js, line 225 at r2 (raw file):

Previously, stefano-maggiolo (Stefano Maggiolo) wrote…

Ditto

Done.


cms/server/contest/templates/contest.html, line 153 at r2 (raw file):

Previously, stefano-maggiolo (Stefano Maggiolo) wrote…

I think you need to translate %d days as some language might invert the order... I'm not sure there is a better way to do it in JS, but then I'd just to a replace("%d", days). @lerks for his extensive localization knowledge.

I don't know if I would want to put this in now. Would you prefer to go this way immediately, or can I do that in a later feature?

Copy link
Member

@stefano-maggiolo stefano-maggiolo left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @eljakim)


cms/server/contest/templates/contest.html, line 153 at r2 (raw file):

Previously, eljakim (Eljakim Schrijvers) wrote…

I don't know if I would want to put this in now. Would you prefer to go this way immediately, or can I do that in a later feature?

Yeah, please go on with the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants