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

LeanCV Review #8517

Closed
iamtakashi opened this issue Dec 11, 2024 · 0 comments
Closed

LeanCV Review #8517

iamtakashi opened this issue Dec 11, 2024 · 0 comments

Comments

@iamtakashi
Copy link
Contributor

iamtakashi commented Dec 11, 2024

Quick summary

Since we found out LeanCV has not yet launched on dotcom, I'm reviewing the theme.

@henriqueiamarino, here are my notes. Since the theme has already been launched in dotorg, I'll list the issues we can fix without disrupting the sites that already use this theme. Let me know your thoughts.

  • duotone-default isn't registered, but avatar, cover, post-featured-image, and site-logo blocks were meant to be used it. Therefore, any page with any of the blocks throws those notices. Ideally, we want to add the working duotone to those blocks, but that will change how those blocks appear on the live sites :( Shall we remove the duotone from those blocks since it has yet to work since the beginning? What do you think? Image

  • I'd recommend turning on the debug mode by editing the wp-config.php to see things like this. Image
    Image

  • Let's turn on the overlay menu for small screens to avoid this. Image

  • It's a CBT fault, but there are strong tags inside the localised string. Let's move the tags outside of the escape function like thisImage

  • I know it's a Gutenberg bug, but let's edit the size label for small, medium, and large font size presets. Image

  • Let's remove unused template parts, parts/header-pages.html and parts/portrait.html. But let's add all the other template parts in theme.json with a title.

  • The footer template part doesn't seem right. Edit the vertical paddings in one of the group blocks, and resaving the theme should fix it.

  • Let's remove unused patters, patterns/page.php, patterns/single.php, patterns/bio.php, patterns/comments.php, patterns/hidden-404.php, patterns/portrait.php, patterns/self.php.

  • A main tag is missing in 404, archives, front page, blog home, index, and search templates.

  • Let's update the Tested up to: to 6.7 in readme.txt and style.css.

  • Let's update the Requires PHP: to 8.2 in readme.txt and style.css.

  • In terms of the theme description, I'm not sure if we want to say this is a blog theme. Also, it's a bit unclear about what "original visuals and interesting navigation" mean here. If you change the description, sync it to the one in style.css.

  • I thought we needed to have the version number like 1.1.0. Let's change it to 1.1.0 in readme.txt and style.css. Then we will bump the version up after this.

  • Remove the following tags: blog, block-patterns, block-styles, custom-colors, custom-header, custom-menu, editor-style, flexible-header, post-formats, and template-editing.

  • Add a wide-blocks tag.

  • There are quite a few uses of numeric font size. I can see lots of 0.8rem, and it's probably a good idea to make a new x-small size and use it for the blocks currently use 0.8rem: comment-author-name, comment-date, comment-edit-link, comment-reply-link, navigation, post-author, post-date, post-terms, site-title, h2, h4, h5, h6.

  • Another numeric font size I saw was 1.5rem, used for post-title and h1. I'm unsure what the best we could do with this. The size is between the medium and the large. Shall we make a new medium-large size, 1.5rem?

Let me know if I can clarify anything further.

Steps to reproduce

A clear and concise description of what you expected to happen.

No response

What actually happened

No response

Impact

All

Available workarounds?

No but the platform is still usable

If the above answer is "Yes...", outline the workaround.

Platform (Simple and/or Atomic)

Self-hosted

Logs or notes

No response

@iamtakashi iamtakashi added [Type] Bug Something isn't working Needs triage labels Dec 11, 2024
@jartes jartes moved this from Needs Triage to Triaged in Automattic Prioritization: The One Board ™ Dec 12, 2024
iamtakashi added a commit that referenced this issue Dec 13, 2024
* LeanCV: fixes issues mentioned on #8517

* LeanCV: Updates theme description on readme and style.css

* LeanCV: removes tags mentioned by ThemeCheck

* Removed unused patterns

* LeanCV: adds multiple duotone presets

* Updated the minimum PHP version

---------

Co-authored-by: Takashi Irie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants