-
Notifications
You must be signed in to change notification settings - Fork 135
Scope module assets #441
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
Closed
Closed
Scope module assets #441
Conversation
This file contains hidden or 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
Author
|
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.jsis 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_Moduleclass. 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_viewwhich simply always returns true, with a//@todoattached to it :) - the whole asset enqueuing process needed a refactor.Introducing
EF_Module_With_ViewThe 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_Moduleclass because all of the modules already are extending it. ButEF_Moduleis also instantiated directly and it’s also extended by aEF_Dashboardclass 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_Moduleand 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_InterfaceandEF_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_Viewthat provides methods to easily determine whether or not assets are neededEF_Module_With_Viewinstead ofEF_Moduleand enqueue assets when they’re neededFixes #351