-
Notifications
You must be signed in to change notification settings - Fork 132
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
Scope module assets #441
Closed
Closed
Scope module assets #441
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
`home_url()` will only add a trailing slash to a site's home URL if a path is passed as an argument. If no path is passed, a page preview could be something like: http://wp.wsu.dev?page_id=1 And it would work fine in most cases. In a subdirectory multisite configuration, a page preview could be something like: http://wp.wsu.dev/site?page_id=1 This could be processed by WordPress as a lookup for a page named "site" rather than a site with that URL because no trailing slash exists. When adding query args to `home_url()`, we should tell it to use a slash to avoid confusion. See `_get_page_link()` for how core builds these in a way where a slash is always added.
fix get_beginning_of_week returning wrong results when given $date is not a start of the week fix get_beginning_of_week returning wrong results on DST switching
All I did was to replace the timesince function with the human_time_diff function from WordPress. This way, the time difference is automatically created.
Even though this method was never implemented - deprecate it, in case someone is relying on it
Rename `disable_custom_statuses_for_post_type` to `is_module_edit_view`
…52-feature/270-comments-notification-meta Revert "Revert "Display who was notified for an editorial comment""
…ated-time Displaying updated time instead of creation time in Notepad Dashboard Widget
Readme updates and version bump 0.8.3
…uttons The green color doesn't make a lot of sense, and has had an ugly blue shadow for a long time. Why not remove the green entirely? It looks good with blue buttons, and won't cause headaches in the future if button-primary changes again.
… be responsive and adapt itself to different resolutions.
You shouldn’t receive a notification email unless you are able to view/edit the post.
$authors is never used, so declaring it then merging in an empty array isn’t needed.
General cleanup for the _get_notification_recipients method to align with WP coding standards. Mostly all spaces, braces, and periods. The one outlier is the removal of the first declaration of $recipients which is not needed.
When array_filter is used without a callback, it will remove any array items that evalulate to false, namely empty strings and NULLs.
…ation-visibility Add a "No Access" badge to subscribers that will not be able to receive notifications
… users/groups who were notified - Updates phpdoc to specify that $comment is WP_Comment - Near the top, we fetch the list of users/groups from comment meta - Before the "actions" list, we output the list of people who were notified Note: This template could use so much love, but for now focusing on putting this new feature in a sensible place, with a plan to return and redesign the template later.
… down below the fake hr
Make @return value more explicit Co-Authored-By: jerclarke <jer@simianuprising.com>
Update `notification_comment()` to escape `$notification_list` before outputting Tested by jer and works Co-Authored-By: jerclarke <jer@simianuprising.com>
…w into scope-module-assets # Conflicts: # modules/calendar/calendar.php # modules/custom-status/custom-status.php # modules/editorial-comments/editorial-comments.php
Oh I messed this one up good, re-opening in #487 :) |
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.
Overview
At the moment, almost all assets are loaded on every admin page (for example -
calendar.js
is loaded in "Settings -> General”). Overall there were a lot of inconsistencies in asset enqueuing. The goal of this PR is to provide a reliable and predictable way of enqueuing module assets.Background
Every Edit Flow module extends the
EF_Module
class. However, some modules have dedicated views and some modules (like Dashboard widgets module) don’t. That’s where the inconsistencies begin.Most EF Modules enqueue assets. Some check whether they should enqueue assets, some perform the same checks in a different way, and some don’t check at all. Most of the modules use method
EF_Module::is_whitelisted_functional_view
which simply always returns true, with a//@todo
attached to it :) - the whole asset enqueuing process needed a refactor.Introducing
EF_Module_With_View
The EF modules needed a few methods to make it easy to check whether the any given module is visible at the moment (and therefore, whether they should enqueue assets). In most cases this is either in the edit, list and settings views.
At first I dumped all the necessary methods on
EF_Module
class because all of the modules already are extending it. ButEF_Module
is also instantiated directly and it’s also extended by aEF_Dashboard
class that doesn’t utilize the any of the “new” methods, so it didn’t make sense to keep them there.EF_Module_With_View extends EF_Module
and is meant to be extended further by modules that have views, that way they have access to necessary methods, likeis_current_module_settings_view()
andis_active_view()
Adding PHP interfaces
There was no streamlined way of dealing with assets. Even enqueuing method names varied from module to module. That’s why I decided to 2 simple interfaces:
EF_Script_Interface
andEF_Style_Interface
- they will ensure that asset enqueuing methods are consistent and predictable. Also, by reading the class declaration it’s immediately clear whether or not the current module is enqueuing any assets.Summary
EF_Module_With_View
that provides methods to easily determine whether or not assets are neededEF_Module_With_View
instead ofEF_Module
and enqueue assets when they’re neededFixes #351