Fix #5701 with SugarTheme Enhancement #6380
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Custom CSS generated for the SuiteP theme was being ignored due to the subTheme system. This enhancement resolves by self-referencing for the current "primary" theme, then descending to get any files for the subTheme.
Codecov Report
@@ Coverage Diff @@
## develop #6380 +/- ##
===========================================
+ Coverage 7.23% 7.23% +<.01%
===========================================
Files 3400 3400
Lines 332825 332828 +3
===========================================
+ Hits 24078 24086 +8
+ Misses 308747 308742 -5
Continue to review full report at Codecov.
|
jack7anderson7
added
the
PR:Community Contribution
gymad
suggested changes
Dec 17, 2018
| @@ -965,12 +965,21 @@ protected function _getImageFileName( | |||
| * @param bool $returnURL if true, returns URL with unique image mark, otherwise returns path to the file | |||
| * @return string path of css file to include | |||
| */ | |||
| public function getCSSURL($cssFileName, $returnURL = true) | |||
| public function getCSSURL($cssFileName, $returnURL = true, $skipSubTheme = FALSE) | |||
There was a problem hiding this comment.
I can not find any usages of third parameter $skipSubTheme. Is this function still works properly if I call it and pass true as $skipSubTheme value? Can I Test is some way? I think this function needs a unit and/or acceptance test to see it works properly in each calling situation.
Also: FALSE should be lowercase as it is in PSR standards what we use:
https://www.php-fig.org/psr/psr-2/#25-keywords-and-truefalsenull
There was a problem hiding this comment.
@gymad The third parameter is used in the following # 970 line?
There was a problem hiding this comment.
@samus-aran Sorry, I was not specific enough. I meant I can not find a single call in the codebase something like this:
$theme->getCSSURL($cssFile, $retUrl, $thirdParameterHere); // "usages of third parameter"
gymad
added
the
Status:Requires Automated Tests
fix last page when modulus is zero
Fixed salesagility#6720 - Remove old recaptcha files
* Update command to allow for cache directory specified in config * Remove warnings/errors when no cached directory present * Update to have namespace "cache" so we can add other cache commands later * Leave alias for current command * Use provided toolsets required by robo for implementation * Print cleaned directories * Confirm before cleaning directory * Allow for force override
…-notes # Conflicts: # modules/Campaigns/Campaign.php # modules/Campaigns/views/view.detail.php # modules/Notes/vardefs.php
…gn-notes Add ability to have Notes on a Campaign
…e-command-config-cache-dir Allow for clean cache command to use config'd cache_dir (plus clean up)
As far as I've been able to tell, this code is entirely unused. So I've removed it.
…l-view-smarty Remove dead code: Re:Remastered
This was only ever used as part of HTMLPurifier (as an experimental parser), as far as I can tell. Since HTMLPurifier is no longer vendored in the repository, this library no longer needs to be vendored either.
…-sax Remove XML_HTMLSax3
Smarty is included via Composer now, so these files can be removed. If anyone is requiring these files in their custom code, they should make sure to remove those requires. All Smarty plugins remain as-is.
The XHProf project has been abandoned for 4 years, and the version vendored here is much older than even that. It's undocumented, unmaintained, and it doesn't seem like it even works on PHP 7.x. "The best way to choose what to keep and what to throw away is to take each item in one’s hand and ask: 'Does this spark joy?' If it does, keep it. If not, dispose of it." - Marie Kondō
It's not used anywhere else in the codebase besides the test suite, should be safe to remove.
These have been deprecated for over six years now. There was only one use of encodeReal, so I've replaced it.
This leaves getHelper() for now because it's still used in a few places. As far as I can tell, all of these have been deprecated since at least the fork from SugarCRM. None of the removed functions are being used elsewhere in the codebase.
This code doesn't do anything and hasn't been used for a long time. Also removes the test for it.
This has been deprecated for a long time and is unused.
…d-sugar-date-time-function Remove get_day_by_index_this_year from SugarDateTime.
…dundant-http-function Remove remove_redundant_http() from Account.
…d-call-helper-function Remove deprecated getReminderTime function in CallHelper.
…d-code-from-dbmanager Remove unused, deprecated functions from DBManager.php.
…d-json-functions Remove deprecated JSON functions encodeReal() and decodeReal()
…d-encrypt-function Remove deprecated encrypt_password function for User.
Remove SugarXHProf
…d-smarty Remove all deprecated Smarty files.
It's been deprecated since SuiteCRM's initial commit and a grep for load_menu doesn't return anything, so it doesn't seem to be used anywhere.
Remove get_admin_modules_for_user, get_theme_display, get_themes, and isValidId. These are all deprecated and unused.
…d-functions Remove deprecated functions from utils.php.
…d-load-menu-function Remove the deprecated load_menu() function in utils.php
Mac-Rae
removed
the
Status:Requires Updates
|
|
mattlorimer
force-pushed
the
develop
branch
from
1998a13
to
7e9bf41
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Custom CSS generated for the SuiteP theme was being ignored due to the subTheme system. This enhancement resolves by self-referencing for the current "primary" theme, then descending to get any files for the subTheme.
Description
Existing customizations from prior versions of Sugar/Suite are using this architecture and it should continue to be supported, while also supporting extra CSS for the Dawn/Dark/etc. subThemes.
Issue logged as #5701
How To Test This
Install a custom CSS file in
custom/themes/SuiteP/css/style.cssthat does something obvious, e.g.QRR and notice that the change isn't seen. Apply the patch and the change will appear because this file is found and used.
Types of changes
Final checklist