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.

Sign up for GitHub

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

Move all inline style to dedicated endpoint #2877

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Move all inline style to dedicated endpoint #2877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/jobs/webstandard.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ if [ "$TEST" = "w3cval" ]; then
unzip -q vnu.linux.zip
section_end

FLTR='--filterpattern .*autocomplete.*|.*style.*|.*role=tab.*|.*descendant.*|.*Stray.*|.*attribute.*|.*Forbidden.*|.*stream.*|.*obsolete.*'
FLTR='--filterpattern .*autocomplete.*|.*role=tab.*|.*descendant.*|.*Stray.*|.*attribute.*|.*Forbidden.*|.*stream.*|.*obsolete.*'
for typ in html css svg
do
section_start "Analyse with $typ"
Expand Down
8 changes: 8 additions & 0 deletions webapp/src/Controller/PublicController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@
return $this->dj->getScoreboardZip($request, $requestStack, $contest, $this->scoreboardService);
}

#[Route(path: '/dynamic-css', name: 'get_dynamic_css')]
Copy link
Member

Choose a reason for hiding this comment

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

The name is too broad. It should be something like scoreboardCategoryColorCss or something.

Copy link
Member Author

Choose a reason for hiding this comment

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

And rename if there are more?

Copy link
Member

Choose a reason for hiding this comment

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

More dynamic CSS? I would put them in separate actions, since otherwise you get one big if here, which doesn't make sense IMHO.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll change it for now, not sure if we have more pages like this so if the discussion will actually ever be non-theoretical.

public function dynamicCSS(): Response {
$response = new Response();
$response->headers->set('Content-Type', 'text/css');
Copy link
Member

Choose a reason for hiding this comment

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

We should maybe set some expire headers? Otherwise browsers won't cache it.
Drawback is that it will change once the colors change, so maybe use an etag header instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

We discussed and I'll try to implement a cache and keep the etag there.

$response->setContent($this->renderView('public/dynamic.css.twig', $this->dj->getDynamicCSS()));
return $response;

Check failure on line 108 in webapp/src/Controller/PublicController.php

View workflow job for this annotation

GitHub Actions / phpcs 

Spaces must be used to indent lines; tabs are not allowed

Check failure on line 108 in webapp/src/Controller/PublicController.php

View workflow job for this annotation

GitHub Actions / phpcs 

Line indented incorrectly; expected at least 8 spaces, found 4
}

/**
* Get the contest from the request, if any
*/
Expand Down
10 changes: 10 additions & 0 deletions webapp/src/Service/DOMJudgeService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use App\Entity\Submission;
use App\Entity\Team;
use App\Entity\TeamAffiliation;
use App\Entity\TeamCategory;
use App\Entity\Testcase;
use App\Entity\User;
use App\Utils\FreezeData;
Expand Down Expand Up @@ -1532,6 +1533,15 @@
return Utils::streamZipFile($tempFilename, 'scoreboard.zip');
}

/**
* @return array{'backgroundColors', array<TeamCategory>}
*/
public function getDynamicCSS(): array {
$backgroundColors = array_map(fn($x) => ( $x->getColor() ?? '#FFFFFF' ), $this->em->getRepository(TeamCategory::class)->findAll());

Check failure on line 1540 in webapp/src/Service/DOMJudgeService.php

View workflow job for this annotation

GitHub Actions / phpcs 

Spaces must be used to indent lines; tabs are not allowed

Check failure on line 1540 in webapp/src/Service/DOMJudgeService.php

View workflow job for this annotation

GitHub Actions / phpcs 

Line indented incorrectly; expected at least 8 spaces, found 4
$backgroundColors = array_merge($backgroundColors, ['#FFFF99']);

Check failure on line 1541 in webapp/src/Service/DOMJudgeService.php

View workflow job for this annotation

GitHub Actions / phpcs 

Spaces must be used to indent lines; tabs are not allowed

Check failure on line 1541 in webapp/src/Service/DOMJudgeService.php

View workflow job for this annotation

GitHub Actions / phpcs 

Line indented incorrectly; expected at least 8 spaces, found 4
return ['backgroundColors' => $backgroundColors];

Check failure on line 1542 in webapp/src/Service/DOMJudgeService.php

View workflow job for this annotation

GitHub Actions / phpcs 

Spaces must be used to indent lines; tabs are not allowed

Check failure on line 1542 in webapp/src/Service/DOMJudgeService.php

View workflow job for this annotation

GitHub Actions / phpcs 

Line indented incorrectly; expected at least 8 spaces, found 4
}

private function allowJudge(ContestProblem $problem, Submission $submission, Language $language, bool $manualRequest): bool
{
if (!$problem->getAllowJudge() || !$language->getAllowJudge()) {
Expand Down
1 change: 1 addition & 0 deletions webapp/templates/base.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

<link rel="stylesheet" href="{{ asset("css/bootstrap.min.css") }}">
<link rel="stylesheet" href="{{ asset("css/fontawesome-all.min.css") }}">
<link rel="stylesheet" href="{{ path('get_dynamic_css') }}">
<script src="{{ asset("js/jquery.min.js") }}"></script>
<script src="{{ asset("js/jquery.debounce.min.js") }}"></script>
<script src="{{ asset("js/bootstrap.bundle.min.js") }}"></script>
Expand Down
18 changes: 0 additions & 18 deletions webapp/templates/partials/scoreboard_table.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,24 +578,6 @@
{% endif %}
{% endif %}

<style>
{% for color,i in backgroundColors %}
{% set colorClass = color | replace({"#": "_"}) %}

.cl{{ colorClass }} {
background-color: {{ color }};
}

{% set cMin = color|hexColorToRGBA(0) %}
{% set cMax = color|hexColorToRGBA(1) %}

.cl{{ colorClass }} .forceWidth.toolong:after {
background: linear-gradient(to right,
{{ cMin }} 0%,
{{ cMax }} 96%);
}
{% endfor %}
</style>
<script>
document.querySelectorAll(".forceWidth:not(.toolong)").forEach(el => {
if (el instanceof Element && el.scrollWidth > el.offsetWidth) {
Expand Down
18 changes: 18 additions & 0 deletions webapp/templates/public/dynamic.css.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{% autoescape false %}
{% for i,color in backgroundColors %}
{% set colorClass = color | replace({"#": "_"}) %}

.cl{{ colorClass }} {
background-color: {{ color }};
}

{% set cMin = color|hexColorToRGBA(0) %}
{% set cMax = color|hexColorToRGBA(1) %}

.cl{{ colorClass }} .forceWidth.toolong:after {
background: linear-gradient(to right,
{{ cMin }} 0%,
{{ cMax }} 96%);
}
{% endfor %}
{% endautoescape %}
Loading