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

Grammer: add theme #7861

Closed henriqueiamarino closed 1 month ago

henriqueiamarino commented 3 months ago

Grammer theme design brings the social media experience to your site and transforms it into a repository for your photos and videos. Demo site

screenshot

github-actions[bot] commented 3 months ago

Preview changes

I've detected changes to the following themes in this PR: Alves, Appleton, Artly, Assembler, Attar, Balasana, Barnsbury, Bitácora, Brompton, Calm Business, Canard, Course, Coutoire, Dalston, Dara, Dorna, Elegant Business, Erma, Eventual, Exford, Farrow, Foam, Fontaine, Friendly Business, George Lois, Grammer, Hall, Heiwa, Hever, Iotix, Jaida, Leven, Loïc, Luminance, Marl, Mayland, Maywood, Meraki, Modern Business, Morden, Muscat, Otis, Pendant, Photos, Professional Business, Redhill, Rivington, Rockfield, Russell, Seedlet, Shawburn, Sophisticated Business, Stow, Stratford, Sunderland, Upsidedown, Varia, Vitrum.

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.

⚠️ Note: Child themes are dependent on their parent themes. You will have to install the parent theme as well for the preview to work correctly.

jasmussen commented 1 month ago

General visual observations

Nice one! Good and simple. Demo site looks solid.

Couple of small things, should the social link treatment be the same in the header and footer?

Screenshot 2024-08-19 at 12 01 32

Screenshot 2024-08-19 at 12 01 37

I wonder if "More about me" could be a placeholder link, rather than include the "bengallo.com".

Screenshot 2024-08-19 at 12 01 14

In general it's always risky to include fake URLs, even if it links to wordpress.org. Might be better without it.

The site when activated looks a bit bare:

Screenshot 2024-08-19 at 12 03 24

This is of course because Playground doesn't include any posts with featured images. The template looks fine, and it's cool you use Grid by the way! It does highlight, however, that the image itself is the only piece linking to a post from the homepage. I wonder if there's a very minimal way to include at least the post-title or link? Could it be a Cover block with a subtle overlay text permalink, and featured background? Or maybe it's fine? Not a blocker.

Mobile looks great.

Template review

Templates look good. I particularly like the post permalink:

Screenshot 2024-08-19 at 12 11 26

Style variations

Style variations feature light and dark modes. That's cool, and works for the photo theme.

As color sets and font sets get increasing footprint in newer versions of WordPress, it'd be nice to make it a routine to include a couple of color palettes for each, some can even repeat between themes with minimal changes. And these can be color-sets or font-sets only, they don't have to be part of the style variations. LMK if you need a rundown.

File and readme reviews

Readme looks good. Though these links don't work (slash replaced by underscore), and it's unclear if they are public domain:

Screenshot 2024-08-19 at 12 13 55

This font is also unclear on the license:

Screenshot 2024-08-19 at 12 14 04

Summary

This one can be ready to go. Looks great. There's a question on omitting "bengallo.com", and a couple of readme updates, otherwise, nice one.

henriqueiamarino commented 1 month ago

Thanks, @jasmussen. These images can be Covers instead; you're right. At least it will change the bare aspect for new users. This one only has images because I have yet another one with them, Cubico.

I'll make sure to iterate following your feedback.

jasmussen commented 1 month ago

The bare aspect for new users is okay. I'm mainly thinking whether a title overlay, even if tiny, can add some value.

henriqueiamarino commented 1 month ago

I want to proceed with images only for the Home page. As mentioned, this theme has an update that has text inside covers so users can choose one or another. Anyway, on the Archive and Search, there are labels following the Featured images that can be used on the Blog Home if one prefers.

Thanks for the feedback @jasmussen; all checked.

jasmussen commented 1 month ago

Looks good, though I'm not sure you had to remove the PT font, I was mainly confused by the license. On deeper investigation, it seems okay to use it? Up to you if you want to add it back. Otherwise, seems good to go.

henriqueiamarino commented 1 month ago

Okay, I added a new font to properly create the style variation instead of only using a color preset. We're good to go now.