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

CoachBen: add theme #8305

Merged
merged 6 commits into from
Oct 31, 2024
Merged

CoachBen: add theme #8305

merged 6 commits into from
Oct 31, 2024

Conversation

henriqueiamarino
Copy link
Collaborator

Coach Ben is a theme designed for coaching professionals. It is tailored for showcasing services, blog posts, and podcasts — the ideal platform to establish a solid online presence and connect coaches with their audience.

Demo site

screenshot

@henriqueiamarino henriqueiamarino added the Ready to launch Add this label if this is the first PR for a new theme label Oct 29, 2024
@henriqueiamarino henriqueiamarino self-assigned this Oct 29, 2024
Copy link
Contributor

Preview changes

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

I'll review this today!

@iamtakashi
Copy link
Contributor

@henriqueiamarino Here are my notes.

  • Let's add a No result block to the archive and the search results templates like the index template.
  • The query type for the query loop block in the search results template should be the default.
    CleanShot 2024-10-30 at 15 24 49@2x
  • The sidebar in the single post template is misaligned. I assume the page about template is what you wanted to achieve. If I'm correct, the block spacing is overlooked.
    CleanShot 2024-10-30 at 15 31 12@2x
  • The page-about template could be called a Page with Sidebar template. Let's add it in the JSON, just like you did for CoachAva.
  • A main tag is missing in the pages template.
  • The single posts template there are two main tags. Only the outer, Content is good.
  • The button colour is in low contrast with the text. Especially in the Lichen and the Tin style variations.
    CleanShot 2024-10-30 at 16 13 04@2x
    CleanShot 2024-10-30 at 16 14 08@2x
  • The font sizes for the button, buttons, post-content, post-terms, and query-pagination have absolute size. None of them are big in size, so it's less likely to become a problem with small viewports, but it'd provide a better UX for customisation if they were all presets.
  • I think wp-brand_7.png is unused. If I'm correct, remove it.
  • Let's edit the body text in the Events section on the homepage template so they are hypothetical.
  • This is a bug in the CBT plugin, but the custom labels in the pagination blocks are left untouched after saving the theme, and we need to make it internationalised manually :/ The steps are:
  1. Create a pattern called pagination.php in the patterns folder and paste this:
<?php
/**
 * Title: pagination
 * Slug: coachben/pagination
 * Inserter: no
 */
?>
<!-- wp:query-pagination {"align":"full","layout":{"type":"flex","justifyContent":"left","flexWrap":"wrap"}} -->
<!-- wp:query-pagination-previous {"label":"<?php esc_html_e( 'Previous', 'coachben' ); ?>"} /-->
<!-- wp:query-pagination-numbers /-->
<!-- wp:query-pagination-next {"label":"<?php esc_html_e( 'Next', 'coachben' ); ?>"} /-->
<!-- /wp:query-pagination -->
  1. The pagination is used in three templates, archive.html, index.php, and search.php, so we need to replace it with the newly created pattern above.
  2. For example, in archive.html, you will delete the pagination block altogether.
    CleanShot 2024-10-30 at 17 32 48@2x
  3. Once you deleted the pagination block, you will call the pattern by <!-- wp:pattern {"slug":"coachben/pagination"} /-->
  4. This is how it should look when you replace it with the pattern.
    CleanShot 2024-10-30 at 17 28 54@2x
  5. Repeat the same in index.php and search.php
  • The comments.php, page-about.php, page.php, and single.php aren't used. Remove them.
  • Tiny detail: Could the name be something else?
    CleanShot 2024-10-30 at 17 51 09@2x
  • Let's add credits to all the fonts bundled in readme.txt
  • This is probably something the CBT used to do at some point, but there is a full SIL font license, let's remove it.
  • Let's add credits to all the images, including the ones you made in the readme.txt.
  • Let's remove irrelevant theme tags. block-styles, custom-colors, custom-header, custom-logo, custom-menu, editor-style, featured-image-header, flexible-header, full-width-template, post-formats, sticky-post, and theme-options.

Let me know if I can clarify anything further!

@henriqueiamarino
Copy link
Collaborator Author

Thanks for the thorough review, @iamtakashi. Here are my fixes:

  • Fixed coach name and description on the sidebar;
  • Made the Search template QL the same as the Archive;
  • Fixed the misalignment on the single template;
  • Renamed the page-about as indicated;
  • Added a main tag to the Page template and removed an extra from the Single;
  • Fixed the button contrast issue: the text color should be the Primary;
  • Removed remaining absolute sizes from elements: button, buttons, post-content, post-terms, and query-pagination;
  • Delete image wp-brand_7;
  • Replaced the copy text of the events section;
  • Fixed and repeated the process of making pagination internationalized for archive.html, index.php, and search.php;
  • Removed the following unused PHP files: comments, page-about, page, and single;
  • Updated style.css tags as required;
  • Added credits related to icons and placeholder logos;
  • Removed SIL font license info and update the missing font information.

@iamtakashi
Copy link
Contributor

iamtakashi commented Oct 31, 2024

@henriqueiamarino Thanks for the update! Most things were addressed correctly 👍

  • You've correctly created the pagination pattern, but the old pagination was still on the templates that needed archive, search, and index templates.
  • No results block wasn't added to the archive and the search templates. But my instruction wasn't sufficient—since the paragraph "Sorry, but nothing [...]" in the no results block needed to be internationalised, the archive template needed to be converted to a pattern, and be called from the archive.html.

I've pushed my edits, 1 and 2 to address the above, take a look at it for the future. Let me know if I can clarify anything further!

This should be ok to merge now.

Copy link
Contributor

Theme-Check results

coachben: No changes required ✅.


@henriqueiamarino
Copy link
Collaborator Author

henriqueiamarino commented Oct 31, 2024

Thanks for the push, @iamtakashi. Sometimes, I don't know what's going on with my fixes. I use Visual Studio Code and save everything correctly, but when you later check, it's not there.
This time, I give you my word: I followed your steps, removed old pagination, and added new to the archive, search, and index templates. Anyway, it's now done.

@henriqueiamarino henriqueiamarino merged commit e9189a5 into trunk Oct 31, 2024
3 checks passed
@henriqueiamarino henriqueiamarino deleted the add/coachben branch October 31, 2024 13:36
@iamtakashi
Copy link
Contributor

You're welcome, @henriqueiamarino.

I wondered what might have happened to the calls to the pagination pattern on the templates. It looks like a call to a pattern adds the markup of the pattern to the editor, even if nothing has changed in the pattern. For example,

Let's say this is in a template file.

<foo>
<!-- wp:pattern {"slug":"coachben/pagination"} /-->
</foo>

That becomes this in the editor.

<foo>
<!-- wp:query-pagination {"align":"full","layout":{"type":"flex","justifyContent":"left","flexWrap":"wrap"}} -->
<!-- wp:query-pagination-previous {"label":"Previous"} /-->
<!-- wp:query-pagination-numbers /-->
<!-- wp:query-pagination-next {"label":"Next"} /-->
<!-- /wp:query-pagination -->
</foo>

So, if you saved the theme with CBT after you added the call to the pattern to the file, the call will be gone. I think this could be what happened. It's annoying. I'll look into whether this is an expected behaviour or not. I don't think this has always been like it. The call to a template part stays in the editor all the time, and that makes sense to me. Anyway, the theme is good to submit to the dotorg now.

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