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

Koinonia: add theme #8470

Merged
merged 5 commits into from
Dec 5, 2024
Merged

Koinonia: add theme #8470

merged 5 commits into from
Dec 5, 2024

Conversation

henriqueiamarino
Copy link
Collaborator

@henriqueiamarino henriqueiamarino commented Nov 28, 2024

Koinonia is a theme for non-profit organizations based on the theme Lativ. The category ‘non-profit’ was selected after a requirement in our list of priorities, while the topics pets and community created an interesting demo site.

Demo site | Content export

screenshot

@henriqueiamarino henriqueiamarino added the Ready to launch Add this label if this is the first PR for a new theme label Nov 28, 2024
@henriqueiamarino henriqueiamarino self-assigned this Nov 28, 2024
@henriqueiamarino henriqueiamarino mentioned this pull request Nov 28, 2024
Copy link
Contributor

Preview changes

I've detected changes to the following themes in this PR: Koinonia.
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.

@iamtakashi iamtakashi self-assigned this Nov 29, 2024
@iamtakashi
Copy link
Contributor

iamtakashi commented Dec 3, 2024

@henriqueiamarino Cute but without being cheesy. Nice work, mate.

  • This section on the homepage is a bit awkward when we see it with a tablet-size viewport because of those misalignments. CleanShot 2024-12-02 at 23 15 31@2x

To combat this, it's just a suggestion, but consider making the layout for the "Intro paragraph" "Services 1,2,3 and 4" default (i.e. not constrained). They don't change with a desktop view, but they all will stretch with a tablet view.

CleanShot 2024-12-02 at 23 35 11@2x

  • This is totally subjective, I'm not sure about the centre-aligned text around the form. Have you considered making it left-aligned or reducing the body text to just one sentence?

CleanShot 2024-12-02 at 23 39 43@2x

  • Let's allow the Navigation row block to wrap to multiple lines, and make the navigation block left-aligned.
    CleanShot 2024-12-02 at 23 48 25@2x

  • For the URLs for each social link, it might just be better to leave it as # or the top page of those social media.

  • This is probably intentional, but the side paddings make the page content misaligned. It also makes the content even narrower on a small screen.

CleanShot 2024-12-03 at 00 12 20@2x
CleanShot 2024-12-03 at 00 36 05@2x

  • Currently, the theme doesn't allow wide and full-width blocks to stretch. Should we allow wide and full-width elements in a content stretch? The one-column layout makes it feel affordable. It's up to you, though.

CleanShot 2024-12-03 at 00 38 43@2x

  • The outline variation of the button block might be odd to some. The difference in the rounded corners between the default and the variation could also be awkward for some.
    CleanShot 2024-12-03 at 14 15 58@2x
  • var(--wp--preset--duotone--tertiary) is not defined but used for the avatar block in JSON.
  • Haskoy Normal 300 is registered twice in the JSON.
  • templates/page.html has two main tags. Let's choose one of them.
  • Can we use the x-small size for the text in the Jetpack contact form?
  • I assume that you made the icons and the speech mark graphics. It's probably better to add the credit to yourself to readme.txt so it's clear for the review team.
  • I'm unsure about including the following theme tags: blog, entertainment, and one-column.
  • I like the theme description includes non-profit organizations. We could also add keywords like pets, animal, dogs, cats etc. It's probably not important to mention the base theme, Lativ.

Let me know if I can clarify further on anything.

@iamtakashi
Copy link
Contributor

One more suggestion. If you want to make the "card" the same height, you can do that by making the height 100%.
CleanShot 2024-12-03 at 15 16 02@2x

@henriqueiamarino
Copy link
Collaborator Author

Thanks, @iamtakashi. Here's what I changed on the latest commit:

  • Fixed the awkward misalignments in the first section of the Homepage;
  • Tested and adjusted the Subscribe block copy;
  • Allowed the Navigation block to wrap to multiple lines;
  • Standardized the social media links with '#';
  • Tried to fix the content wrapper to allow wide and full blocks (not sure if I did it);
  • Fixed the avatar duotone from Tertiary to Default.
  • Fixed Haskoy font in theme.json
  • Deleted an extra tag from the Page template;
  • Set Jetpack form text as on the Home.php pattern;
  • Added line of credits related to icons and graphic elements to readme.txt;
  • Remove unnecessary tags from style.css
  • Added new terms to the theme description;
  • Made the post template blocks 100% to have cards aligned.

@iamtakashi
Copy link
Contributor

iamtakashi commented Dec 4, 2024

@henriqueiamarino, thanks for the update. I pushed a commit to address these things.

  • Added a font file for the Haskoy light italic, which was missing.
  • A little tweak to the single posts template so that the wide and the full-width elements will stretch.
  • Added the new theme description to style.css. There are two places we need to have the theme description. style.css and readme.txt.
  • Reformatted the theme tags. They need to be all small cases, and no space is allowed. https://make.wordpress.org/themes/handbook/review/required/theme-tags/

I think the theme is now ready to launch. Ship it! 🚢

Copy link
Contributor

github-actions bot commented Dec 4, 2024

Theme-Check results

koinonia: No changes required ✅.


@henriqueiamarino
Copy link
Collaborator Author

Thanks, @iamtakashi. Let me check the correct way to proceed with this before the merge. Last time, I pressed Squash and merge, expecting your changes to be added, but I think they were lost.

Should I use one of the Update options before continuing?

Screenshot 2024-12-05 at 11 10 21

@iamtakashi
Copy link
Contributor

If no further change is pushed to this branch, you can squash and merge. I have already made changes to this branch, which will be deleted when you squash and merge.

What might have happened last time was that you made the change on top of the local copy of the theme, which didn't pull my change correctly for some reason.

@henriqueiamarino henriqueiamarino merged commit e8fdeb8 into trunk Dec 5, 2024
3 checks passed
@henriqueiamarino henriqueiamarino deleted the add/koinonia branch December 5, 2024 11:04
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.

2 participants