-
Notifications
You must be signed in to change notification settings - Fork 223
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
Closes #7069: 3.18 - Exclusions part #7167
base: feature/host-google-fonts
Are you sure you want to change the base?
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
I might be mistaken but I'm not sure this PR considers this thread here and the proposals following because as it is we might not be correctly excluding combined fonts or not? |
@remyperona @jeawhanlee I've made modifications to comply with feedbacks. Summary of changes by this PR:This pull request introduces several enhancements to the font optimization process, specifically focusing on excluding certain fonts from being optimized based on user-defined patterns. These changes include updates to the core functionality, new methods for exclusion handling, and corresponding test cases. Core Functionality Enhancements:
Integration with Dynamic Lists:
Google Fonts Optimization:
Test Cases: |
22258dd
to
ef79267
Compare
@Miraeld, thank you for this PR. Returning this back to In Progress based on this comment. cc @piotrbak |
@hanna-meda after discussion on the daily, since the URL doesn't matter for the exclusions, we'll match the regex after it:
cc @jeawhanlee |
Description
Fixes #7069
It allows the user to use a new filter
rocket_exclude_locally_host_fonts
to exclude some fonts from the feature.Type of change
Detailed scenario
Just add some fonts to exclusiosn through the new filter (
rocket_exclude_locally_host_fonts
), and it won't be downloaded.Technical description
Documentation
This pull request introduces a new feature to exclude specific fonts from being rewritten in the media fonts optimization process. The changes involve adding methods to handle font exclusions and updating relevant tests to ensure the new functionality works as expected.
New Feature: Font Exclusions
inc/Engine/Media/Fonts/Frontend/Controller.php
: Added methodsget_exclusions
andis_excluded
to obtain and check font exclusion patterns, and updatedrewrite_fonts
to skip excluded fonts. [1] [2]inc/Engine/Optimization/DynamicLists/DynamicLists.php
: Added methodget_exclude_media_fonts
to retrieve excluded font templates.inc/Engine/Optimization/DynamicLists/Subscriber.php
: Subscribed to therocket_media_fonts_exclusions
event and addedadd_media_fonts_exclusions
method to include media font exclusions. [1] [2]Tests and Fixtures Updates
tests/Fixtures/inc/Engine/Media/Fonts/Frontend/Subscriber/HTML/expected_v1_excluded_v2.php
: Added a new fixture for testing the exclusion of V2 fonts while rewriting V1 fonts.tests/Fixtures/inc/Engine/Media/Fonts/Frontend/Subscriber/rewriteFonts.php
: Added a new test casetestShouldRewriteV1AndExcludeV2Fonts
to verify the exclusion functionality.tests/Integration/inc/Engine/Media/Fonts/Frontend/Subscriber/rewriteFonts.php
: Updated setup and teardown methods to include the newrocket_exclude_locally_host_fonts
filter, and added theexclude_locally_host_fonts
method. [1] [2]New dependencies
None
Risks
None
Mandatory Checklist
Code validation
Code style