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

Colorloops: add theme #8130

Merged
merged 6 commits into from
Sep 26, 2024
Merged

Colorloops: add theme #8130

merged 6 commits into from
Sep 26, 2024

Conversation

scruffian
Copy link
Member

Changes proposed in this Pull Request:

Related issue(s):

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

github-actions bot commented Sep 13, 2024

Preview changes

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

@henriqueiamarino henriqueiamarino marked this pull request as draft September 24, 2024 11:40
@henriqueiamarino henriqueiamarino marked this pull request as ready for review September 24, 2024 13:45
@henriqueiamarino henriqueiamarino changed the title Add: Colorloops Colorloops: add theme Sep 24, 2024
@iamtakashi
Copy link
Contributor

iamtakashi commented Sep 25, 2024

@henriqueiamarino Very cool aesthetic!

  • I'd recommend that each query loop block on the front page template not include sticky posts, especially in this theme, where multiple query loop blocks are used. We don't see sticky posts in the editor, but the front of the site actually shows them and the multiple sticky posts seem like a bug. I believe this is an editor bug, and it's filed in here and there.

CleanShot 2024-09-25 at 15 32 38@2x

CleanShot 2024-09-25 at 15 34 27@2x

  • The post titles on the index template are tiny compared to other templates. It seems like an oversight.
  • There is a single-line break tag in the heading on the 404 template. Do you actually want a line break there?
  • I'd recommend not using "lorem ipsum" filler in a theme template/template part. It feels less intentional. Something small, like a made-up paragraph, would be much nicer.

CleanShot 2024-09-25 at 16 05 54@2x

  • The following patterns are not used anywhere in the theme. They should be removed.
    • colorloops/hidden-no-results-content
    • colorloops/separator
    • colorloops/comments
    • colorloops/page

Let me know if I need to clarify anything!

@iamtakashi
Copy link
Contributor

Sorry, I forgot to add. This probably has something to do with the bug in the CBT plugin, but the name WordPress in the footer credit is no longer a link. I needed to fix it manually in Cubico. See here. But it's annoying that next time you save the theme, the HTML appears. This is also a bug in CBT. I'll add a comment again in the issue.

@henriqueiamarino
Copy link
Collaborator

Thanks, @iamtakashi. I checked and fixed everything you mentioned.
I also removed the unused Patterns, although I believe CBT has been adding them to all themes.

@iamtakashi
Copy link
Contributor

Thanks for the update. I see a single line break in the paragraph on the sidebar. Everything else looks good.
CleanShot 2024-09-26 at 14 17 32@2x

I believe CBT has been adding them to all themes.

No, they are patterns from the base theme you worked on to make this theme. At this time, we need to manually remove those unused patterns inherited from the base theme after exporting the theme.

Copy link
Contributor

Theme-Check results

colorloops: No changes required ✅.


@henriqueiamarino henriqueiamarino merged commit d5699bb into trunk Sep 26, 2024
3 checks passed
@henriqueiamarino henriqueiamarino deleted the add/colorloops branch September 26, 2024 15:26
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