Skip to content
This repository has been archived by the owner on Oct 28, 2022. It is now read-only.

add pilgrimage theme styles #166

Merged
merged 8 commits into from
Sep 19, 2022
Merged

Conversation

kathryncodes
Copy link
Contributor

addresses #134

Please let me know if any changes are needed and I will be sure to get them in the next couple days. Thanks!

@madhusudhand
Copy link
Collaborator

@kathryncodes A style variation doesn't need to duplicate all styles from main theme.json because they are combined to produce final results. Include the style definitions that are different from the main theme.json.
As an example, color palette, core-navigation -> elements -> color etc.. are the changes this variation can have.

Here is a similar variation definition #157 for your reference.

@kathryncodes
Copy link
Contributor Author

@madhusudhand ah okay, I was actually trying to referencing that file but I suppose I didn't need to include everything in it. This file is what I got from using the block theme plugin on my local environment, but I can trim it down. Would your suggestion be to simply remove any code that's the same as theme.json?

@carolinan
Copy link
Collaborator

Would your suggestion be to simply remove any code that's the same as theme.json?

Yes

@kathryncodes
Copy link
Contributor Author

Latest commit has all the duplicate theme.json styles removed from pilgrimage.json

@carolinan @madhusudhand thank you for being so helpful!

@mikachan mikachan mentioned this pull request Sep 17, 2022
10 tasks
@mikachan mikachan linked an issue Sep 17, 2022 that may be closed by this pull request
@mikachan
Copy link
Member

Thanks for working on this! ❤️ It's looking great. I've just pushed a few commits to fix up some formatting, colours and to add the variation title. I've also added a subtle background gradient as mentioned in this comment.

@beafialho
Copy link
Collaborator

I'm not seeing the intended duotone in the images:

Captura de ecrã 2022-09-19, às 10 43 11

@mikachan
Copy link
Member

I'm not seeing the intended duotone in the images:

Oh, good spot! It looks like we were missing the duotone filter. I've added this and applied it to all images.

@madhusudhand
Copy link
Collaborator

Also, should the background be gradient? I refer single color background base from figma.

@mikachan
Copy link
Member

Also, should the background be gradient? I refer single color background base from figma.

This was requested in this comment here, but it would be great to know if this was the intended effect!

Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

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

In order to include this variation in Beta 1, I'm going to merge this now. Please feel free to open any additional PRs to make any changes!

@mikachan mikachan merged commit c373046 into WordPress:trunk Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variation: Pilgrimage
5 participants