-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix: avoid useless loading of scripts #64
base: release/v2.5.0
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good.
My only suggestion would be to consider using an allowed list rather than an excluded list of pages. There are probably fewer pages for the allow list and we can be sure that over time that this is easier to manage.
@circlecube thanks for your suggestion. Before creating the pull request I discussed about that with @arunshenoy99 and we agree that set a list of pages where include the script is risky because the image optimized feature works on media library and it could be managed by a third-part plugins which works outside the list of pages declared. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me. Will get this QA’d once before merging. Thanks!
Proposed changes
Added a code to load conditionally bulk image optimizer scripts according the page which is loaded, and the admin style only in case visit the plugin panel page.
Type of Change
Production
Development
Video
Checklist
Further comments