URL watcher notification changes #10089
Conversation
admin/class-yoast-notification.php
Outdated
| @@ -62,6 +62,7 @@ class Yoast_Notification { | |||
| 'dismissal_key' => null, | |||
| 'capabilities' => array(), | |||
| 'capability_check' => self::MATCH_ALL, | |||
| 'yoast-branding' => false, | |||
There was a problem hiding this comment.
I believe the - should be a _.
admin/class-yoast-notification.php
Outdated
| if ( $this->options['yoast-branding'] ) { | ||
| $message = $this->wrap_yoast_seo_icon( $this->message ); | ||
| } | ||
| if ( ! isset( $message ) ) { |
There was a problem hiding this comment.
Missing whitespace above this line.
admin/class-yoast-notification.php
Outdated
| @@ -282,8 +283,36 @@ public function render() { | |||
| // Combined attribute key and value into a string. | |||
| array_walk( $attributes, array( $this, 'parse_attributes' ) ); | |||
|
|
|||
| if ( $this->options['yoast-branding'] ) { | |||
There was a problem hiding this comment.
See previous comment regarding the usage of -.
admin/class-yoast-notification.php
Outdated
| * | ||
| * @param string $message The message to wrap. | ||
| * | ||
| * @return string |
| 60, | ||
| 60 | ||
| ); | ||
| $out .= '<div class="yoast-seo-icon-wrap">'; |
There was a problem hiding this comment.
Why not make these three lines part of the sprintf?
There was a problem hiding this comment.
I decided not to fix this, because the sprintf is about the image tag being parsed
| * | ||
| * @param string $first_sentence The first sentence of the notification. | ||
| * | ||
| * @return string |
| protected function get_message( $first_sentence ) { | ||
| return '<h2>' . __( 'Make sure your visitors don\'t get errors!', 'wordpress-seo' ) . '</h2>' | ||
| . '<p>' | ||
| /* translators: %1$s expands to the translated name of the post type, %2$s expands to the anchor opening tag, %3$s to the anchor closing tag. */ |
There was a problem hiding this comment.
I believe this comment won't be parsed properly by gettext.
| /** | ||
| * Adds a notification to be shown on the next page request since posts are updated in an ajax request. | ||
| * | ||
| * @param string $post_type_label The singular_name label from a post_type_object. | ||
| * @param string $message The singular_name label from a post_type_object. |
There was a problem hiding this comment.
Is this documentation still correct as the variable name changed to $message?
| ), array( 'type' => 'notice-info' ) | ||
| $message, array( | ||
| 'type' => 'notice-warning is-dismissible', | ||
| 'yoast-branding' => true, |
There was a problem hiding this comment.
See previous comment regarding the usage of - vs _.
| @@ -41,6 +41,7 @@ public function test_set_defaults() { | |||
| 'dismissal_key' => null, | |||
| 'capabilities' => array( 'wpseo_manage_options' ), | |||
| 'capability_check' => 'all', | |||
| 'yoast-branding' => false, | |||
There was a problem hiding this comment.
See previous comment regarding the usage of - vs _.
|
Acceptance done. I'm not sure how I feel about the notification. When you delete a post, you get redirected to the posts overview. This is fine, however the notification states "You just trashed this Post [...]". My immediate reaction to such a message was "What post exactly?". I'm fairly certain I won't be the only person that thinks this. |
|
Acceptance done. Will check with @jdevalk about suggested copy change. |
Summary
This pull includes the changes in #10077, but makes some changes:
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
This PR can be tested by following these steps:
Quality assurance
Fixes https://github.com/Yoast/wordpress-seo-premium/issues/1798