-
Notifications
You must be signed in to change notification settings - Fork 224
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
3.18 - Exclusions part #7069
Comments
This is dependent on the completion of #7063 Scope a solution:We'll create a filter
We add our new filter to the subscriber class
In
Estimation |
I think it doesn't matter whatever is passed to the filter, be it a full url or a part of it like the name, we should be able to exclude it using regex, we've done something similar in the past with other features, having 2 filters could seem redundant for accomplishing the same thing. |
I have a question, does the backend exclusion mentioned mean having a textarea in our settings page on the admin or rather handling externally, the same way we do with the dynamic lists @piotrbak @Khadreal @MathieuLamiot |
@jeawhanlee It's only filter + backend for now |
Thanks for spotting it @jeawhanlee |
@jeawhanlee the grooming has been updated. |
LGTM |
The current grooming is missing to consider how should exclusions work when the Google fonts link is composed of multiple fonts? Does it mean we need to rewrite the google link to keep the excluded font? So splitting the combine into 2 separate links, with only one being processed. How does the exclusion interacts with the combine google fonts feature? |
@remyperona What about excluding the font from being combined? Then it'll not be combined and might not be part of the rewrite system. |
What about the case where the fonts are already combined, not by our feature? |
Then, if our exclusion matches, we'll exclude whole combined (not by us) CSS file |
Initially picked by @Khadreal but since he is off, I am moving this back to ToDo. |
Co-authored-by: Opeyemi Ibrahim <[email protected]> Co-authored-by: Rémy Perona <[email protected]> Co-authored-by: Michael Lee <[email protected]>
User Story
As a user, I’d like to be able to exclude fonts from the feature using filter
Acceptance Criteria
The text was updated successfully, but these errors were encountered: