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.

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

Add Midnight variation #268

Merged
merged 13 commits into from
Sep 24, 2024
Merged

Add Midnight variation #268

merged 13 commits into from
Sep 24, 2024

Conversation

beafialho
Copy link
Contributor

Description

Adds the Midnight variation. I had to make adjustments that may look different from the Figma file.

I tried to get the section styles to have the optimal color contrast, but these may need further refinement that's beyond my technical knowledge.

I also added the specific "Midnight" duotone effect for this variation.

There's an extra left border added to the quote and pullquote blocks, I don't know why this is happening, but it shouldn't be there. If someone could help me find that, I'd appreciate it.

Screenshots

midnight.mp4

Copy link

github-actions bot commented Sep 11, 2024

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: beafialho <[email protected]>
Co-authored-by: carolinan <[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.

@richtabor
Copy link
Member

Let's hold off on the big combined variations until the singular color sets and typesets are 100% solid. That way we don't have to redo anything.

@carolinan
Copy link
Contributor

carolinan commented Sep 14, 2024

Why would they not be 100% solid?

@juanfra juanfra added the [Priority] High Used to indicate top priority items that need quick attention label Sep 16, 2024
Copy link

github-actions bot commented Sep 22, 2024

Preview changes

You can preview these changes by following the link below:

I will update this comment with the latest preview links as you push more changes to this PR.
⚠️ Note: The preview sites are created using WordPress Playground. You can add content, edit settings, and test the themes as you would on a real site, but please note that changes are not saved between sessions.

@carolinan
Copy link
Contributor

I believe the quote and pullquote border issues were solved when the testimonial patterns were merged.

Default:
quotes-default

Midnight:
quotes-midnight

@carolinan
Copy link
Contributor

carolinan commented Sep 22, 2024

  • Section 3 is identical to the default colors of the variation. It might not be an issue, but perhaps it could be adjusted so that the section could be taken advantage of.

@carolinan
Copy link
Contributor

carolinan commented Sep 22, 2024

Should the color changes be made to the palette preset too?

Midnight palette Midnight global style
Accent 2 #79F3B1 New Accent 2 #372696
Primary #D594F4 New Primary #79f3b1
Secondary #372696 New Secondary #e8b7ff

I am confused by why base and accent 1 needed to be the same value.

@carolinan
Copy link
Contributor

carolinan commented Sep 22, 2024

Section Style 2 uses the text color #372696 (accent 2) over the background color #251D51 (accent 3) and this has a contrast ratio of 1.36:1. It needs to be 4.5:1 (3:1 for large text).

My suggestion would be to use the pink/purple text color #e8b7ff (secondary) because it would give a contrast ratio of 9.21:1.
Let me know if I can implement this.

What looks like a border in the image below is the default body background color, I just cropped the image badly :)

image

The button style does not match the one in Figma but from reading the comments, I don't know if this is on purpose.

@carolinan
Copy link
Contributor

carolinan commented Sep 22, 2024

In Section 4, the outline button is invisible. yet another reason to remove it... #382

@carolinan
Copy link
Contributor

carolinan commented Sep 22, 2024

In section 5, the text color is #79F3B1 (contrast) and the background #e8b7ff (secondary).
This gives a contrast ratio of 1.2:1.

This style is not in Figma. I think this is meant to be section 2 but inverted, and that the text color is meant to be the dark purple, #251D51 (accent 3). Let me know if I can implement this.
But either of the dark blues: base or accent 2, also works.

@carolinan
Copy link
Contributor

carolinan commented Sep 24, 2024

The button block styles are overriding the elements button styles, including :hover, so I need to do a lot of adjustments to the buttons.

@carolinan
Copy link
Contributor

carolinan commented Sep 24, 2024

I find that I need to add this for all states:

			"section-1": {
				"elements": {
					"button": {
						":hover": {
							"color": {
								"background": "var:preset|color|accent-2",
								"text": "var:preset|color|contrast"
							},
							"border": {
								"color": "var:preset|color|contrast"
							}
						}
					}
				}
			},

And I think that these should be in the file for section-1, but it will affect every other combined variation.

Ensure that the default filled button has the text, background, and border colors reverted on hover.
@carolinan carolinan merged commit 2c8f915 into trunk Sep 24, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Style Variations [Priority] High Used to indicate top priority items that need quick attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants