Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Subscriptions Block: Remove inline styling for colors on status message #19230

Merged
merged 5 commits into from
Mar 25, 2021
Merged

Subscriptions Block: Remove inline styling for colors on status message #19230

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ccac402
Remove inline styling for colors on subscription status message
stacimc Mar 19, 2021
061e3ea
Only set border color when defined in theme
stacimc Mar 22, 2021
45208cd
Add changelog
stacimc Mar 22, 2021
148c3ef
Merge remote-tracking branch 'origin/master' into fix/remove-inline-s…
jeherve Mar 24, 2021
6a0a96b
Clean up with printf
stacimc Mar 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 14 additions & 21 deletions projects/plugins/jetpack/modules/subscriptions/views.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,43 +210,36 @@ static function render_widget_status_messages( $instance ) {

if ( self::is_wpcom() && self::wpcom_has_status_message() ) {
global $themecolors;
$message = '';

switch ( $_GET['blogsub'] ) {
case 'confirming':
echo "<div style='background-color: #{$themecolors['bg']}; border: 1px solid #{$themecolors['border']}; color: #{$themecolors['text']}; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;'>";
_e( 'Thanks for subscribing! You&rsquo;ll get an email with a link to confirm your subscription. If you don&rsquo;t get it, please <a href="https://en.support.wordpress.com/contact/">contact us</a>.' );
echo "</div>";
$message = __( 'Thanks for subscribing! You&rsquo;ll get an email with a link to confirm your subscription. If you don&rsquo;t get it, please <a href="https://en.support.wordpress.com/contact/">contact us</a>.', 'jetpack' );
break;
case 'blocked':
echo "<div style='background-color: #{$themecolors['bg']}; border: 1px solid #{$themecolors['border']}; color: #{$themecolors['text']}; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;'>";
_e( 'Subscriptions have been blocked for this email address.' );
echo "</div>";
$message = __( 'Subscriptions have been blocked for this email address.', 'jetpack' );
break;
case 'flooded':
echo "<div style='background-color: #{$themecolors['bg']}; border: 1px solid #{$themecolors['border']}; color: #{$themecolors['text']}; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;'>";
_e( 'You already have several pending email subscriptions. Approve or delete a few through your <a href="https://subscribe.wordpress.com/">Subscription Manager</a> before attempting to subscribe to more blogs.' );
echo "</div>";
$message = __( 'You already have several pending email subscriptions. Approve or delete a few through your <a href="https://subscribe.wordpress.com/">Subscription Manager</a> before attempting to subscribe to more blogs.', 'jetpack' );
break;
case 'spammed':
echo "<div style='background-color: #{$themecolors['bg']}; border: 1px solid #{$themecolors['border']}; color: #{$themecolors['text']}; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;'>";
echo wp_kses_post( sprintf( __( 'Because there are many pending subscriptions for this email address, we have blocked the subscription. Please <a href="%s">activate or delete</a> pending subscriptions before attempting to subscribe.' ), 'https://subscribe.wordpress.com/' ) );
echo "</div>";
/* translators: %s is a URL */
$message = sprintf( __( 'Because there are many pending subscriptions for this email address, we have blocked the subscription. Please <a href="%s">activate or delete</a> pending subscriptions before attempting to subscribe.', 'jetpack' ), 'https://subscribe.wordpress.com/' );
break;
case 'subscribed':
echo "<div style='background-color: #{$themecolors['bg']}; border: 1px solid #{$themecolors['border']}; color: #{$themecolors['text']}; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;'>";
_e( 'You&rsquo;re already subscribed to this site.' );
echo "</div>";
$message = __( 'You&rsquo;re already subscribed to this site.', 'jetpack' );
break;
case 'pending':
echo "<div style='background-color: #{$themecolors['bg']}; border: 1px solid #{$themecolors['border']}; color: #{$themecolors['text']}; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;'>";
_e( 'You have a pending subscription already; we just sent you another email. Click the link or <a href="https://en.support.wordpress.com/contact/">contact us</a> if you don&rsquo;t receive it.' );
echo "</div>";
$message = __( 'You have a pending subscription already; we just sent you another email. Click the link or <a href="https://en.support.wordpress.com/contact/">contact us</a> if you don&rsquo;t receive it.', 'jetpack' );
break;
case 'confirmed':
echo "<div style='background-color: #{$themecolors['bg']}; border: 1px solid #{$themecolors['border']}; color: #{$themecolors['text']}; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;'>";
_e( 'Congrats, you&rsquo;re subscribed! You&rsquo;ll get an email with the details of your subscription and an unsubscribe link.' );
echo "</div>";
$message = __( 'Congrats, you&rsquo;re subscribed! You&rsquo;ll get an email with the details of your subscription and an unsubscribe link.', 'jetpack' );
break;
}

echo sprintf( "<div style='border: 1px solid #%s; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;'>", esc_attr( $themecolors['border'] ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During my tests (using Spearhead) I noticed that, where $themecolors['border'] is empty, the following invalid CSS is output:

border: 1px solid #; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;

Do you think it'd be better to only print the inline style border: 1px solid #; if $themecolors['border'] is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed 👍 I kept the 1px solid border but removed the empty color value, rather than omitting the border altogether. The border takes on the text-color in this case.

Screen Shot 2021-03-22 at 2 05 52 PM

Per the discussion below -- once we can access the theme border color with CSS variables, we can move all of the styling out into the stylesheets and it will be much cleaner :)

echo wp_kses_post( $message );
echo '</div>';
}
}

Expand Down