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

Rainfall: Add paddings and other visual theme improvements #8551

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

beafialho
Copy link
Collaborator

Rainfall is currently looking badly because it has no paddings on the site at all:

rainfall.mp4

I went ahead and fixed it, along with other visual improvements I thought should be done to the theme overall.

@iamtakashi when you have a moment, can you review these changes? Also, let me know if there's anyone else I should ask for a review. Thanks in advance.

@beafialho beafialho added [Type] Bug Something isn't working [Theme] Rainfall labels Dec 19, 2024
@beafialho beafialho requested a review from iamtakashi December 19, 2024 15:56
Copy link
Contributor

Preview changes

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

iamtakashi commented Dec 19, 2024

@beafialho Thanks for the PR! The changes look good, but there seem to be extra side paddings around the title in the archive and the page with black header templates.
CleanShot 2024-12-19 at 20 21 14@2x

Also, for some reason, the height of the spacers in this change seems broken. Example
You can fix them if you re-save the templates with CBT after setting the height in the editor. If you go to the files changed tab and search \u002d, you should be able to find which spacers are broken.
CleanShot 2024-12-19 at 23 11 57@2x

And "ref":1909 is added back in the navigation block in this change. That makes the preview broken. The CBT should automatically remove that when we save it with CBT.
CleanShot 2024-12-19 at 20 40 55@2x

Let me know if you need help with anything.

@beafialho
Copy link
Collaborator Author

Thanks for reviewing @iamtakashi! I submitted a fix for the spacer issues and the extra side paddings you pointed out.

Regarding the navigation block, I can't find the "ref":1909 bit anywhere in the theme files, where do you see that?

But yes, CBT was supposed to handle that.. I don't understand why that's happening because I clicked in this option when I saved it:

Captura de ecrã 2024-12-20, às 09 27 46

@beafialho
Copy link
Collaborator Author

Correction: I spotted one ref in the footer part. Submitted a fix for that too!

Copy link
Contributor

Theme-Check results

rainfall: No changes required ✅.


@iamtakashi
Copy link
Contributor

Thanks for the update, @beafialho.

There are still broken spacers in parts/query.html(which is used in the archive and search templates) and templates/single.html. Could you check all the spacers used in the single posts template?

There are some templates with images and text that need to be localised. Can you make sure the Localize Text and Images options are checked, and the Process Only Modified Templates option is unchecked the next time you save the theme?
CleanShot 2024-12-20 at 13 28 41@2x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Theme] Rainfall [Type] Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants