Make WordPress Core

Opened 18 months ago

Closed 14 months ago

Last modified 14 months ago

#55981 closed defect (bug) (fixed)

Twenty Nineteen: Pullquote Block Text Color not reflected on frontend

Reported by: nithins53's profile nithins53 Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch
Focuses: css Cc:

Description

On Twenty Nineteen theme, when we add the Pullquote block, and add color to the text, it is only reflected on the editor and not on the front end.

Steps to Replicate:

  1. Activate the Twenty Nineteen theme
  2. Insert Pullquote block
  3. Enter text and add color

For more information, screen recording link attached below.

https://www.loom.com/share/74752e102b50468fb38d591949d93c0e

Thank You.

Attachments (5)

55981.patch (447 bytes) - added by umesh84 18 months ago.
added patch
55981.diff (1002 bytes) - added by deepakvijayan 18 months ago.
Fix pullquote text color not updating issue (patch for sass as well)
55981.2.diff (1.4 KB) - added by deepakvijayan 18 months ago.
Fix pullquote text color not updating issue (patch for sass as well) + style-rtl.css
55981.3.diff (653 bytes) - added by poena 14 months ago.
Move the text color to the wrapper
pullquote-blocks-2019-with-compiled-stylesheets.png (159.2 KB) - added by sabernhardt 14 months ago.
front end, with compiled stylesheets

Download all attachments as: .zip

Change History (28)

@umesh84
18 months ago

added patch

#1 @sabernhardt
18 months ago

  • Component changed from Themes to Bundled Theme
  • Focuses css added
  • Milestone changed from Awaiting Review to Future Release

Thanks for the report!

This had been fixed on #49410, but now the color class is added to the .wp-block-pullquote figure instead of the blockquote element.

Perhaps each color in _blocks.scss could include a selector like .wp-block-pullquote.has-primary-color blockquote.

    .has-primary-color,
    .wp-block-pullquote blockquote.has-primary-color,
    .wp-block-pullquote.has-primary-color blockquote,
    .wp-block-pullquote.is-style-solid-color blockquote.has-primary-color,
    .wp-block-pullquote.is-style-solid-color blockquote.has-primary-color > p {
        color: $color__link;
    }

(The Solid Color block style seems unavailable now, so that may not need adjustment.)

#2 follow-up: @sabernhardt
18 months ago

  • Keywords has-patch needs-refresh added; needs-patch removed

Oops, I missed the patch. If we use inherit instead of my suggestion, a patch still needs to include changes to the appropriate SCSS file (and both style.css and style-rtl.css).

Last edited 18 months ago by sabernhardt (previous) (diff)

@deepakvijayan
18 months ago

Fix pullquote text color not updating issue (patch for sass as well)

@deepakvijayan
18 months ago

Fix pullquote text color not updating issue (patch for sass as well) + style-rtl.css

#3 in reply to: ↑ 2 @nithins53
18 months ago

Replying to sabernhardt:

Oops, I missed the patch. If we use inherit instead of my suggestion, a patch still needs to include changes to the appropriate SCSS file (and both style.css and style-rtl.css).

The new patch by Deepak Vijayan seems to cover what you have mentioned.

#4 @nithins53
18 months ago

@sabernhardt 55981.2.diff covers both the SCSS files.

#5 @sabernhardt
18 months ago

  • Keywords needs-refresh removed
  • Milestone changed from Future Release to 6.1

Thanks for the patch! Removing the color from the blockquote element works with trunk and the latest Gutenberg. Then it falls back to the body color (also #111) when no special colors are specified.

However, there was a reason to add it in the first place. The editor's theme.css used to make the block a gray color, which was changed to `currentColor` for Gutenberg 9.4.

.wp-block-pullquote {
  color: #40464d;
}

To accommodate older versions of WordPress, Twenty Nineteen probably could move that color from the blockquote to the block container.

	.wp-block-pullquote {
		color: $color__text-main;
		border-color: transparent;
		border-width: 2px;
		padding: $size__spacing-unit;

		blockquote {
			border: none;
			margin-top: calc(4 * #{ $size__spacing-unit});
			margin-bottom: calc(4.33 * #{ $size__spacing-unit});
			margin-right: 0;
			padding-left: 0;
		}

#6 @nithins53
18 months ago

@sabernhardt Thank you for testing it out.

@poena
14 months ago

Move the text color to the wrapper

#7 @poena
14 months ago

In 55981.3, I implemented the change that @sabernhardt suggested and moved the text color.

This worked well for me in 5.9.4 and 6.1-beta2-54351.

I found one exception, when the pull quote has the black background color from the palette, the theme enforces a white text color:

.editor-styles-wrapper .has-dark-gray-background-color p {
    color: #fff;
}

I assume this was added to maintain a high color contrast, -but before there were text color options.
Do we consider this a bug or feature?

Last edited 14 months ago by poena (previous) (diff)

This ticket was mentioned in Slack in #core by chaion07. View the logs.


14 months ago

#9 @chaion07
14 months ago

  • Keywords dev-feedback added

@nithins53 thank you for reporting this. We reviewed this ticket during a recent bug-scrub session. Based on the feedback received from the team we are making the following changes:

  1. Adding keyword dev-feedback for visibility
  2. We need to consider the exception mentioned by @poena
  3. A feedback from the core committers would be highly appreciated

Cheers!

Props to @nithins53 @cu121 @robinwpdeveloper

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


14 months ago

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


14 months ago

#12 @audrasjb
14 months ago

  • Keywords commit added; dev-feedback removed

As per today's bug scrub:

Looking at comment:7, I think we can probably mark this one as ready for commit. The proposed patch fixes the issue, and the small exception doesn’t look like a bug to me, it’s an exception, and maybe even a feature.

#13 @audrasjb
14 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

#14 @audrasjb
14 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 54413:

Twenty Nineteen: Ensure Pullquote Block text color is reflected on front-end.

This changeset ensures Pullquote Block text is consistent between the editor a on front-end.

Props nithins53, umesh84, sabernhardt, deepakvijayan, poena.
Fixes #55981.

#15 @sabernhardt
14 months ago

  • Keywords needs-patch added; has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

This still needs the compiled stylesheets :)

This ticket was mentioned in PR #3426 on WordPress/wordpress-develop by whaze.


14 months ago
#16

  • Keywords has-patch added; needs-patch removed

#17 @whaze
14 months ago

hi
i add a PR with compiled stylesheets from https://core.trac.wordpress.org/changeset/54413

#18 @audrasjb
14 months ago

Thanks @whaze!
@poena or @sabernhardt are you available to review this follow-up patch? (I have a lot on my plate today).

@sabernhardt
14 months ago

front end, with compiled stylesheets

#19 @sabernhardt
14 months ago

The patch fixes the reported problem: compiled stylesheets add the correct text color for quote text within the post content.

We may want to revisit this, however, on another ticket. The citation is always gray in the post content, and theme color classes do not apply outside of .entry-content (for any blocks).

Last edited 14 months ago by sabernhardt (previous) (diff)

#20 @audrasjb
14 months ago

  • Keywords commit added

Thanks a lot @sabernhardt!

Yes a new ticket to address further issues in the next major would be a nice to have :)

#21 @audrasjb
14 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 54466:

Twenty Nineteen: Add missing compiled CSS declarations after [54413].

Follow-up to [54413].

Props sabernhardt, whaze.
Fixes #55981.

#23 @sabernhardt
14 months ago

  • Keywords commit removed

For the follow-up tickets, we can probably use #56455 and #56456.

Note: See TracTickets for help on using tickets.