-
Notifications
You must be signed in to change notification settings - Fork 62
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
Functionality placeholders #1981
Conversation
Related to #1980
This needs to be DRY'd and improved, but it's looking pretty good.
Traits allow us to not add too much into the abstract plugin and theme hooks class. To make a class function as a placeholder, you add `use GravityView_Functionality_Placeholder;` then implement the abstract methods.
@zackkatz I'm unsure if it's included, but please don't forget to include the PDF for GravityView tab. |
@doekenorg Since you have recently worked on this for #1996, I think it makes sense if you were to work on the functionality. Let me know if you have questions. |
- Extracted a placeholder.php template - Replaced `add_placeholder_hooks` with a more generic `add_inactive_hooks` when the plugin is inactive. - Removed the trait and opted for a Placeholder value object - Added `extra_nav_class` option to a metabox for displaying purposes. - Added a `gk/navigation/tab` hook to overwrite the navigtaion label HTML. - Added hooks before and after a metabox for additional rendering purposes.
It looks better!
This handles multi-rule strings while also sanitizing
assets/js/admin-views.js
Outdated
const ajaxRoute = $( this ).data( 'action' ) + '_product'; | ||
const text_domain = $( this ).data( 'text-domain' ); | ||
|
||
// Todo: refactor after #1996 is merged. |
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.
@zackkatz or @doekenorg, what kind of refactoring is expected here?
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.
Good question! That's one for @doekenorg I believe.
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.
I think this has to do with code duplication when it comes to activating plugins. See if we can reuse the logic from that PR instead having it here again.
Implements #1980
💾 Build file (729b388).