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

Changes about platform style theme - refs BT#21621 #5614

Merged
merged 16 commits into from
Jul 5, 2024

Conversation

AngelFQC
Copy link
Member

No description provided.

Copy link

codecov bot commented Jun 26, 2024

Codecov Report

Attention: Patch coverage is 33.16062% with 129 lines in your changes missing coverage. Please review.

Project coverage is 39.06%. Comparing base (77064d7) to head (54e6796).
Report is 3 commits behind head on master.

Files Patch % Lines
...e/Migrations/Schema/V200/Version20231110194300.php 0.00% 21 Missing ⚠️
...e/Migrations/Schema/V200/Version20240704185300.php 10.00% 18 Missing ⚠️
src/CoreBundle/Entity/AccessUrlRelColorTheme.php 0.00% 17 Missing ⚠️
src/CoreBundle/Entity/AccessUrl.php 34.78% 15 Missing ⚠️
src/CoreBundle/Entity/ColorTheme.php 0.00% 13 Missing ⚠️
...dle/State/AccessUrlRelColorThemeStateProcessor.php 0.00% 13 Missing ⚠️
src/CoreBundle/ServiceHelper/ThemeHelper.php 80.00% 7 Missing ⚠️
...ndle/State/AccessUrlRelColorThemeStateProvider.php 0.00% 7 Missing ⚠️
src/CoreBundle/State/ColorThemeStateProcessor.php 0.00% 6 Missing ⚠️
...e/Migrations/Schema/V200/Version20240318105600.php 33.33% 4 Missing ⚠️
... and 6 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5614      +/-   ##
============================================
- Coverage     39.07%   39.06%   -0.02%     
- Complexity    10989    11024      +35     
============================================
  Files           867      872       +5     
  Lines         45411    45477      +66     
============================================
+ Hits          17746    17767      +21     
- Misses        27665    27710      +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

*/
function api_get_visual_theme()
function api_get_visual_theme(): string
Copy link

Choose a reason for hiding this comment

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

Consider putting global function "api_get_visual_theme" in a static class

src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
@AngelFQC AngelFQC force-pushed the BT21621_theme branch 2 times, most recently from b671ba8 to 3984206 Compare June 27, 2024 14:18
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
@AngelFQC AngelFQC changed the title Changes about platform style theme Changes about platform style theme - refs BT#21621 Jun 28, 2024
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
return $this->colorThemes;
}

public function addColorTheme(AccessUrlRelColorTheme $colorTheme): static
Copy link

Choose a reason for hiding this comment

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

Scope keyword "static" must be followed by a single space

src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
return $this;
}

public function removeColorTheme(AccessUrlRelColorTheme $colorTheme): static
Copy link

Choose a reason for hiding this comment

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

Scope keyword "static" must be followed by a single space

src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
@AngelFQC AngelFQC force-pushed the BT21621_theme branch 2 times, most recently from 3a57e11 to f0d9e9a Compare July 1, 2024 16:18
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved
src/CoreBundle/Entity/AccessUrl.php Show resolved Hide resolved

/* For licensing terms, see /license.txt */

declare(strict_types=1);
Copy link

Choose a reason for hiding this comment

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

Add a single space around assignment operators

@AngelFQC AngelFQC force-pushed the BT21621_theme branch 2 times, most recently from e346837 to d186d97 Compare July 1, 2024 23:04
@AngelFQC AngelFQC marked this pull request as ready for review July 4, 2024 23:38
Copy link

codeclimate bot commented Jul 5, 2024

Code Climate has analyzed commit 54e6796 and detected 191 issues on this pull request.

Here's the issue category breakdown:

Category Count
Style 179
Clarity 9
Bug Risk 3

View more on Code Climate.

@@ -9,30 +9,30 @@
<table border="0" cellpadding="0" cellspacing="0">
<tr>
<td colspan="3">
<img src="{{ url('index') ~ 'build/css/themes/' ~ stylesheet_name ~ '/images/cuadro.png' }}">
<img src="{{ theme_asset('images/cuadre.png') }}">
Copy link
Member

@ywarnier ywarnier Jul 5, 2024

Choose a reason for hiding this comment

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

Not sure where cuadre.png or cuadro.png come from. I cannot see them as files in a default installation... (not even in 1.11.x)

Copy link
Member

Choose a reason for hiding this comment

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

These are the files used for the general certificate from OFAJ (which is custom as indicated in the name of the template).

@ywarnier ywarnier merged commit bb0a675 into chamilo:master Jul 5, 2024
4 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants