xwp / travel

WordPress AMP Adventures Themes (Native AMP)
GNU General Public License v3.0
34 stars 6 forks source link

36 - Single product template #39

Closed mehigh closed 6 years ago

mehigh commented 6 years ago

This is left:

And of course copying the product.css from ampstart (i've setup that repo next to this and referenced the CSS with a regular CSS link during development). Reached 44KB of CSS until now.

single-product

miina commented 6 years ago

Started adding theme integration Currently (within 9d60672) using placeholders for header.php, footer.php, and static comments.php from @mehigh's created HTML template.

Maybe comments.php can be implemented within this PR since that's directly related to the CPT template but perhaps proper header template(s) and footer template could be separate PR-s/issues? Thoughts, @kienstra and @postphotos?

kienstra commented 6 years ago

Header And Footer Templates

Hi @miina,

...perhaps proper header template(s) and footer template could be separate PR-s/issues

That's a good idea. Separate issues and PRs for header.php and footer.php would help organize this.

And we could merge the templates into develop as soon as they're ready, instead of waiting for all of them.

DavidCramer commented 6 years ago

@miina - please take a look at #38 as it could affect the structures.

miina commented 6 years ago

@kienstra Forgot to mention that this PR also fixes #8 (Ratings logic).

kienstra commented 6 years ago

Building editor-blocks.js

The develop branch removes editor-blocks.js, in order to avoid merge conflicts in PRs. So I also removed that file from this branch to remove the conflict to develop.

Please do npm run build when checking out this branch to use the Gutenberg blocks.

mehigh commented 6 years ago

The html/css is complete for the page now: laptop-view phone-view

miina commented 6 years ago

@kienstra Thanks for reviewing, most of the issues should be addressed now. Left the gallery as static for now, hopefully that's OK for now and could be addressed later if needed. There's just one issue that I'm not sure how to fix, any thoughts?

@mehigh Thanks for generating the HTML! There's one CSS issue that would be great if you could help with -- namely within the Similar Posts section there's a div with class travel-overflow-container. The CSS for the class in the adventure.css looks like this:

.travel-overflow-container {
    display: inline-block;
    min-width: 100vw;
    padding-left: calc((100vw - 72rem) / 2);
    padding-right: calc((100vw - 72rem) / 2);
}

However, with the AMP plugin on it looks like the padding gets stripped and only this is left:

.travel-overflow-container {
    display: inline-block;
    min-width: 100vw;
}

Would be great if you could help with that issue.

postphotos commented 6 years ago

Making sure we capture - related to #36.