-
Notifications
You must be signed in to change notification settings - Fork 351
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
Layover: add theme #7890
Layover: add theme #7890
Conversation
Preview changesI've detected changes to the following themes in this PR: Alves, Appleton, Artly, Assembler, Balasana, Barnsbury, Bitácora, Brompton, Calm Business, Canard, Cookbook, Course, Coutoire, Dalston, Dara, Elegant Business, Erma, Eventual, Exford, Foam, Fontaine, Friendly Business, George Lois, Hall, Heiwa, Hever, Iotix, Jaida, Layover, Leven, Loïc, Mayland, Maywood, Meraki, Modern Business, Morden, Muscat, Otis, Pendant, Photos, Professional Business, Redhill, Rivington, Rockfield, Seedlet, Shawburn, Sophisticated Business, Stow, Stratford, Sunderland, Upsidedown, Varia, Vitrum. 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. |
The |
General visual observations This is a favorite of mine, looks fantastic. It's a bit hard to review fully, though, as I'm also not seeing the background image you're referring to. But I'm not actually seeing any Cover block at all in the Home template: Feel free to hit me up in chat and we can review sync if need be, maybe screenshare, see where it might be missing or what I should check. Other than that, virtually everything looks great. Good on mobile, no glitches, templates nice. Style variations are a bit bare. The muted colors are nice, but I'm missing both some font alternatives, and perhaps a light/dark monochrome styles. Not blockers, mainly food for thought. I did notice one awkwardly named font: In fact, there are quite a lot of fonts in the font folder, but in testing the templates, it looks like only a single sans-serif font is used. Maybe you did add fonts for the style variations, but they're not working fully as intended? Something to look at. File and readme reviews Readme looks good. Summary Great theme, will be a nice one to land. We need to fix the missing image issue, and figure out what's up with the fonts. Other than that, this one is close. |
Thanks, @jasmussen. I fixed as much as I could and improved the style variations. I need help fixing the home template, though. CBT plugin replaces the image file with a URL from the demo site and fails to load it. |
@@ -6,8 +6,8 @@ | |||
* Inserter: no | |||
*/ | |||
?> | |||
<!-- wp:cover {"url":"https://localhost.local/wp-content/uploads/2024/08/layover_background.jpg","hasParallax":true,"dimRatio":0,"customOverlayColor":"#212121","isUserOverlayColor":true,"contentPosition":"center center","style":{"spacing":{"margin":{"top":"0rem","bottom":"0rem"},"padding":{"top":"0rem","right":"0rem","bottom":"0rem","left":"0rem"}}},"layout":{"type":"constrained"}} --> | |||
<div class="wp-block-cover has-parallax" style="margin-top:0rem;margin-bottom:0rem;padding-top:0rem;padding-right:0rem;padding-bottom:0rem;padding-left:0rem"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-0 has-background-dim" style="background-color:#212121"></span><div class="wp-block-cover__image-background has-parallax" style="background-position:50% 50%;background-image:url(https://localhost.local/wp-content/uploads/2024/08/layover_background.jpg)"></div><div class="wp-block-cover__inner-container"><!-- wp:template-part {"slug":"header","align":"wide"} /--> | |||
<!-- wp:cover {"url":"<?php echo esc_url( get_stylesheet_directory_uri() ); ?>/assets/images/layover_background.jpg","hasParallax":true,"dimRatio":0,"customOverlayColor":"#212121","isUserOverlayColor":true,"contentPosition":"center center","style":{"spacing":{"margin":{"top":"0rem","bottom":"0rem"},"padding":{"top":"0rem","right":"0rem","bottom":"0rem","left":"0rem"}}},"layout":{"type":"constrained"}} --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a change manually to fix this issue. Not sure why CBT didn't fix this: did we do something wrong in the export process, @matiasbenedetto? The image landed in the right directory, it just wasn't attached here.
With the latest fix, I think this is ready to merge! Nice work. |
Layover displays large titles and excerpts that scroll over an image on the Homepage and neat single pages for users who want their blogging to be simple.
Demo site