Automattic / themes

Free WordPress themes made by Automattic for WordPress.org and WordPress.com.
https://themeshaper.com
GNU General Public License v2.0
887 stars 355 forks source link

Maverick: add theme #7866

Closed henriqueiamarino closed 3 months ago

henriqueiamarino commented 3 months ago

This is a theme in homage to George Lois, visionary American Art Director and author. We will all miss your big ideas. Rest in Peace, George. Demo site

screenshot

github-actions[bot] commented 3 months ago

Preview changes

I've detected changes to the following themes in this PR: Maverick.

You can preview these changes by following the links 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.

jasmussen commented 3 months ago

Meta note, due to a crash, I lost a big review I had written here 😩, as a result, the second version that I'm writing below here might be shorter. Sorry about that.

General visual observations

Looks good. If you can include demo sites, that might be good, this one can feel a bit bare with just the default WordPress content. But even looking at the screenshot makes it clear what the expression is that this theme is going for: screenshot

And that's a good look.

The search block might benefit from a different treatment without borders:

Screenshot 2024-06-26 at 09 36 10

The typography looks like it can be bold, but without some sample content, it's hard to review.

Screenshot 2024-06-26 at 09 36 27

Maybe the site title can have the same allcaps treatment in header and footer?

Screenshot 2024-06-26 at 09 39 10

Style variations

The light style variation looks good too!

I mentioned in another review that coming to Gutenberg are separate color and typography variations. Although you don't have to use those, it's a reminder that style variations can be more than just color changes. They can be font, spacing changes too.

File and readme reviews

I noticed in the style.scss file that there are some local overrides. I appreciate for each of those that you include a link to a GB issue so eventually we can minimize this.

Summary

Overall looks good. I'd love to give it another look in case you have some demo content handy, and in general (future themes, if not this one), it would be good to look at how to upgrade the style variations a bit, at least mix up the typography in addition to the colors.

Thanks for your work!

henriqueiamarino commented 3 months ago

Thanks, @jasmussen, I'll address all your observations individually:

henriqueiamarino commented 3 months ago

And @jasmussen, I agree with all your observations on the Summary, but I don't have time to do so now. I am creating numerous PRs, submitting and editing old issues that keep returning, and I feel the massive need to create new designs. Because that is what people expect from me. It is what I feel people care about, at least from the feedback I receive.

jasmussen commented 3 months ago

Thanks for sharing the demo content. It looks great.

The feedback I'm sharing here is not a blocker for this PR, feel free to move forward. These are mainly observations for new future themes to think about. Or, to tinker with if a chance presents itself to revisit this PR. So you can run with this!

henriqueiamarino commented 3 months ago

@jasmussen, I created a new style variation by inverting the color choices and the typefaces. I also fixed some minor details here and there. Unfortunately, now I have these conflicts and must figure out how to deal with them.

That said, this version is the final one; I'll discard this and create a new PR with no conflicts.

jasmussen commented 3 months ago

Sounds good. Is this PR still valid to review? Or should I review a different one? In any case you can just ping me on whatever PR I shoul review next, otherwise I'll just continue with the list.

henriqueiamarino commented 3 months ago

You can continue with the list. This will be a problematic one. Following Takashi's recommendations, I accidentally merged it when I updated it last time. I have to learn how not to do it. Anyway, I just discovered there's a Maverick theme already in Dotorg, which is not present here where we check. I'll have to rename it and start over this process.

After that, I will ask somebody to help me delete it from the trunk.

jasmussen commented 3 months ago

Sounds good, you can just ping me on any new PR when you're ready!