Modal headings and labels missing editor text color

[ghorivipul97]

#50448

What?

Added Default Color of Model heading & Labels

Why?

Default theme color and editor text color are different so need to apply common colour for all model

How?

Apply default color class to model edior

Testing Instructions

• Open any Model
• Check default color of label and heading

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @ghorivipul97! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[mikachan]

👋 @ghorivipul97, thank you for working on this! It looks like you may need to run npm run test:unit -- -u to update the test snapshots, and then commit any updates to this PR. This will hopefully allow the tests to pass.

[mikachan]

It looks like the heading color has not been applied, but I can see the additional class has been added to the modal container:

Trunk	This PR

It seems that the desired color #1e1e1e is being overridden by #1d2327. We perhaps need to add this style somewhere more specifically, maybe somewhere like this: https://github.com/WordPress/gutenberg/blob/trunk/packages/components/src/modal/style.scss#L92

[ghorivipul97]

It looks like the heading color has not been applied, but I can see the additional class has been added to the modal container:

Trunk This PR

It seems that the desired color #1e1e1e is being overridden by #1d2327. We perhaps need to add this style somewhere more specifically, maybe somewhere like this: https://github.com/WordPress/gutenberg/blob/trunk/packages/components/src/modal/style.scss#L92

okay

[stokesman]

Thanks for working on this!

I think we might want to petition for some design scrutiny here because it does affect more than just text colors. The margin and size of subheadings are also affected. A comparison:

(The smaller and more spaced out headings is this PR).

Because the class/styles from the Interface package are making the difference I'd want to think it’s good and helps with consistency. Although, I'd not say that spacing is an improvement.

I think we might consider moving this forward by only affecting color and leaving the rest for another issue.

[stokesman]

Suggested change

	color: $gray-900;
	color: $components-color-foreground;

In the components package this is the appropriate variable to apply for color (so that it can be themed).

[stokesman]

This is the change that also brings in font-size and margin differences to the subheadings. Another thing to note is this won't correct the color issues noted in other modals like the one for Block locking mentioned in the issue. Those modals aren't based on this component. I'd be interested in evaluating some other options.

One option might be to simply add a rule for color here:

gutenberg/packages/interface/src/components/preferences-modal-section/style.scss

Lines 13 to 17 in 6e8cba0

	.interface-preferences-modal__section-title {
	font-size: 0.9rem;
	font-weight: 600;
	margin-top: 0;
	}

Another option if we do want all the font-size and margin differences is to convert the styles of interface-complementary-area into a mixin that can also be used on interface-preferences-modal because I think that'd be cleaner than doubling up on class names from the interface package on this element.

Finally if it's not considered too prescriptive there's a rule we could add to the base Modal component that would obviate the need to change anything in the Interface package and also cover the other modals. That's something like this:

.components-modal__frame * {
	color: $components-color-foreground;
}

I'm not sure that would get sign-off from the component crew but it'd be the most succinct way to fix this.

[Mamaduka]

Hi, @ghorivipul97

Would you still like to work on this PR and address the feedback? Otherwise, we can close this as "stale".

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ghorivipul97 <[email protected]>
Co-authored-by: mikachan <[email protected]>
Co-authored-by: stokesman <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: falgunihdesai <[email protected]>
Co-authored-by: richtabor <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[juanfra]

Thank you for working on this. Closing as fixed in #71311