Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 7 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 2 years ago.
added patch
55981.diff (1002 bytes) - added by deepakvijayan 2 years ago.
Fix pullquote text color not updating issue (patch for sass as well)
55981.2.diff (1.4 KB) - added by deepakvijayan 2 years ago.
Fix pullquote text color not updating issue (patch for sass as well) + style-rtl.css
55981.3.diff (653 bytes) - added by poena 2 years ago.
Move the text color to the wrapper
pullquote-blocks-2019-with-compiled-stylesheets.png (159.2 KB) - added by sabernhardt 2 years ago.
front end, with compiled stylesheets

Download all attachments as: .zip

Change History (29)

@umesh84
2 years ago

added patch

#1 @sabernhardt
2 years 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
2 years 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 2 years ago by sabernhardt (previous) (diff)

@deepakvijayan
2 years ago

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

@deepakvijayan
2 years ago

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

#3 in reply to: ↑ 2 @nithins53
2 years 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
2 years ago

@sabernhardt 55981.2.diff covers both the SCSS files.

#5 @sabernhardt
2 years 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
2 years ago

@sabernhardt Thank you for testing it out.

@poena
2 years ago

Move the text color to the wrapper

#7 @poena
2 years 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 2 years ago by poena (previous) (diff)

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


2 years ago

#9 @chaion07
2 years 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.


2 years ago

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


2 years ago

#12 @audrasjb
2 years 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
2 years ago

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

#14 @audrasjb
2 years 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
2 years 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.


2 years ago
#16

  • Keywords has-patch added; needs-patch removed

#17 @whaze
2 years ago

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

#18 @audrasjb
2 years ago

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

@sabernhardt
2 years ago

front end, with compiled stylesheets

#19 @sabernhardt
2 years 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 2 years ago by sabernhardt (previous) (diff)

#20 @audrasjb
2 years 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
2 years 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
2 years ago

  • Keywords commit removed

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

#24 @sabernhardt
7 months ago

#56567 was marked as a duplicate.

Note: See TracTickets for help on using tickets.