Apply global theme styles to default blog posts #2727
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lougoncharenko
requested review from
sofiamatulis,
ludoboludo,
kmeleta and
Oliviammarcello
1 task
ludoboludo
reviewed
Jun 22, 2023
ludoboludo
reviewed
Jun 22, 2023
ludoboludo
reviewed
Jun 22, 2023
kmeleta
reviewed
Jun 23, 2023
There was a problem hiding this comment.
Looks good! The duplicate card markup of course isn't ideal, but doesn't look as bad as I thought. At least we get to remove the old placeholder styles. Let's just address the comments Ludo has up and I'll look to approve after. Also thank you for the helpful PR description/testing steps 👌
Oliviammarcello
approved these changes
Jun 23, 2023
kmeleta
approved these changes
Jun 26, 2023
ludoboludo
approved these changes
Jun 27, 2023
| style=" {% if settings.blog_card_style == 'standard' %} --ratio-percent: 100%;{% elsif settings.blog_card_style == 'card' %} --ratio-percent: 0%;{% endif %}" | ||
| > | ||
| <div class="card__inner{% if settings.blog_card_style == 'standard' %} color-{{ settings.blog_card_color_scheme }} gradient{% endif %} ratio" | ||
| style="--ratio-percent: 80%;" |
There was a problem hiding this comment.
Any specific reason why we use 80% here instead of 100% ?
Here I set one image to use 100% to compare what it looks like next to the 80%:
There was a problem hiding this comment.
when I spoke with @Oliviammarcello, she preferred the 80 :)
lougoncharenko
deleted the
apply-global-theme-styles-to-default-blog-posts
branch
phapsidesGT
pushed a commit
to Gravytrain-UK/gt-shopify-dawn-theme
that referenced
this pull request
Sep 3, 2024
* applied global theme settings * global theme settings applied, image sized to match other blog posts, preset settings work * fixed spacing for stylesheet * removing change to settings * applied changes suggested in code review * applied suggested changes from code review --------- Co-authored-by: Ludo <ludo.segura@shopify.com>
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Summary:
Default blog cards are responsive to presets and can be altered by number of blog posts to show, number of columns, color scheme, section paddings and show featured image.
Blog posts theme settings can be applied to default blog posts.
The placeholder image's height and width on default blog posts match other blog posts.
Why are these changes introduced?
Fixes #2688
What approach did you take?
I used the article card and card class names to make the default blog cards responsive to the blog card theme settings and removed the blog placeholder specific classes that weren't responsive to the theme settings.
Other considerations
Visual impact on existing themes
This new change will not impact existing merchant stores but will improve the user experience when adding a featured blog.
Testing steps/scenarios
Blog card theme settings
Demo links
Checklist