-
Notifications
You must be signed in to change notification settings - Fork 116
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
Theme Edits Grayed Out #1658
base: develop
Are you sure you want to change the base?
Theme Edits Grayed Out #1658
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.
A couple of comments but this looks good. I looked it up and the ajax method was added in 4.9 so I think we're safe? It will still work with < WP 4.9 but won't record the changes.
Should we add a "Requires at least" header? I'm not really sure which version that would be though.
$this->edited_file = $this->get_plugin_data( $plugin_slug ); | ||
} | ||
|
||
$this->log_changes( $location ); |
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.
❓question
If there's no $location
here, should we log anything? I don't think so?
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.
Fixed that.
) { | ||
$action = wp_stream_filter_input( INPUT_POST, 'action' ); | ||
$request_method = wp_stream_filter_input( INPUT_SERVER, 'REQUEST_METHOD' ); | ||
|
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.
❗️issue
Can you add a nonce and capability check here? I think it can be the same capability check as is required to do the action.
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.
Fixed that as well.
Fixes #1657
Use alternative hook to track the theme files updates in the Dashboard.
Checklist
contributing.md
).Release Changelog