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

Streamfield wrapper overhaul and featured tiles style #101

Merged

Conversation

liamjohnston
Copy link

@liamjohnston liamjohnston commented Jun 14, 2024

Hide whitespace changes when reviewing this.

This PR does two sort of related things.

Firstly: adds a style (background-color & padding) and when a standard tiles block is "featured":
Screenshot 2024-06-14 at 12 20 27 PM

Secondly: this was the catalyst for finally refactoring the streamfield blocks to not auto-magically render wrapping divs that don't exist in any template, e.g. .block-accordion_block.
While it seems like I'm adding a bunch of extra divs here, I'm essentially just renaming the divs that were already rendering, but with a bit more consistency/control. Now we don't need to style tags that have no corresponding html in the codebase.

*/

.block-newsletter {
.block--newsletter {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most part I'm not styling these .block--x classes in terms of component styling. Mainly just for streamfield-wrapper spacing and alignment.

Newsletter is a special case because I want to keep the DOM light, as it's also used as a non-streamfield block in the footer.

@@ -34,7 +43,7 @@
row-gap: 40px;

@include sm {
grid-template-columns: repeat(auto-fit, minmax(px2rem(280), 1fr));
grid-template-columns: repeat(auto-fill, minmax(px2rem(280), 1fr));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops. This meant that tiles block with only one tile had a big full-width tile, which we don't want. I always get these two confused.

@@ -48,6 +57,7 @@
.tile__text {
grid-row: 2;
grid-column: 1;
background-color: var(--color-white);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the component bg can now be grey. But the tile should always have a white bg.

@@ -2,7 +2,7 @@
Regarding content migrated as part of Springload's 2024 rebuild.
Existing content was dumped into a "migrated" content streamfield block.
*/
.migrated-content {
.block--migrated-content {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another special case. I mean, I could add another wrapper but I don't intend to add any styling for this content at this stage.

Comment on lines +11 to +13
{% for block in page.body %}
{% include_block block %}
{% endfor %}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the method that stops rendering the auto-generated wrapping divs. I have applied it everywhere we loop over streamfields.

@liamjohnston liamjohnston requested review from haydngreatnews, sarahframe and a team June 14, 2024 00:59
@liamjohnston liamjohnston merged commit 15d390d into main Jun 14, 2024
3 of 5 checks passed
@liamjohnston liamjohnston deleted the feature/streamfield-wrappers-and-featured-tile-style branch June 14, 2024 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants