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

Margarethe: Add new theme #8083

Merged
merged 13 commits into from
Sep 11, 2024
Merged

Margarethe: Add new theme #8083

merged 13 commits into from
Sep 11, 2024

Conversation

MaggieCabrera
Copy link
Contributor

@MaggieCabrera MaggieCabrera commented Sep 6, 2024

pNEWy-hXf-p2

screenshot

Copy link
Contributor

github-actions bot commented Sep 6, 2024

Preview changes

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

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.

@MaggieCabrera MaggieCabrera added the Ready to launch Add this label if this is the first PR for a new theme label Sep 6, 2024
@MaggieCabrera
Copy link
Contributor Author

This one is ready for review, it should be ready to submit to dotorg

@henriqueiamarino
Copy link
Collaborator

@MaggieCabrera I checked the following items, and they are all LGTM:

  • Templates and elements;
  • Skip links;
  • Hard-coded lines (with Theme Check)
  • Readme file

I found only two topics that can be problematic:

Theme Check alerted us about these three images that can be compacted:

WARNING: assets/images/margarethe-img-5-scaled.jpg is 590.1 KB in size. Large file sizes have a negative impact on website performance and loading time. Compress images before using them.
WARNING: assets/images/margarethe-img-4-scaled.jpg is 591.0 KB in size. Large file sizes have a negative impact on website performance and loading time. Compress images before using them.
WARNING: assets/images/margarethe-img-2-scaled.jpg is 767.5 KB in size. Large file sizes have a negative impact on website performance and loading time. Compress images before using them.

The Theme URI doesn't follow our standards. We have been using showcase pages on the Themeshaper site. So, this theme's page should be:
https://themeshaper.com/margarethe/

@MaggieCabrera MaggieCabrera added Ready to launch Add this label if this is the first PR for a new theme and removed Ready to launch Add this label if this is the first PR for a new theme labels Sep 10, 2024
Copy link
Contributor

@alaczek alaczek left a comment

Choose a reason for hiding this comment

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

This looks good to me. The only thing I flagged is the size of some of the images - I don't know what's the recommended size, but I think anything over 350-400KB needs to be compressed.

@MaggieCabrera
Copy link
Contributor Author

I made the changes to the images and the theme URL, I'm bringing this in, thanks for the reviews!

@MaggieCabrera MaggieCabrera merged commit c7e7a6c into trunk Sep 11, 2024
2 checks passed
@MaggieCabrera MaggieCabrera deleted the add-margarethe branch September 11, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to launch Add this label if this is the first PR for a new theme
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants