Add Protocol article styles, with centered title #258
Conversation
|
|
||
| .mzp-c-article { | ||
| margin: 0 auto; | ||
| max-width: $content-lg; |
There was a problem hiding this comment.
This is actually a bit too wide, and makes for a less than optimal line length for readability. If we set it to $content-md it's more readable, but would change current instances of articles on future.m.o... if we're OK with making that change I'd like to do it, but didn't take the liberty just yet. It doesn't breaking anything and does look better, and is how it should have been all along, but it will change alignments with other components on the page. So not sure how to proceed...
There was a problem hiding this comment.
Can we hack some CSS on actual live pages via devtools to see what the 'damage' is like?
| @@ -553,7 +553,7 @@ class Meta: | |||
| def frontend_media(self): | |||
| "Custom property that lets us selectively include CSS" | |||
| return forms.Media( | |||
| css={"all": [static("css/birdbox-article.css")]}, | |||
| css={"all": [static("css/protocol-article.css")]}, | |||
There was a problem hiding this comment.
Doesn't this mean we lose the bb-specific styling from src/css/article.scss? I think we'll be using that somewhere in production, I imagine
There was a problem hiding this comment.
We can include both CSS files, if that's not gonna leave us with clashing classes. Indeed, I think it won't clash because the current stuff in birdbox-article.css is prefixed with bb-
There was a problem hiding this comment.
I think we can and should keep 'em both - protocol core stuff + bb-only customisation.
| css={"all": [static("css/protocol-article.css")]}, | |
| css={"all": [static("css/protocol-article.css"), static("css/birdbox-article.css")]}, |
There was a problem hiding this comment.
The birdbox-article is being used for blog posts, and the regular Protocol article is being used for regular article blocks. I regret the confusing naming, and we could rename the blog one to something more descriptive like "blogpost" but that would require a migration.
Blog pages were new and non-Protocol so ended up getting their own style sheet, I just foolishly named it the same thing as the Protocol component, though with a different namespaced class.
|
|
||
| .mzp-c-article { | ||
| margin: 0 auto; | ||
| max-width: $content-lg; |
There was a problem hiding this comment.
Can we hack some CSS on actual live pages via devtools to see what the 'damage' is like?
Description
Looks like we weren't loading any of the article component styles for article blocks, and were loading the blog post styles instead... none of which were being applied anyway because they all hang from a different class. This adds a separate Protocol article style for regular (non-blog) article blocks, and also includes the class for centered titles.
It overrides a few aspects of the original component (the width and margins), but the component is slated for some changes in upstream Protocol soonish anyway.
CHANGELOG.md.Testing
Within an article block, set the title alignment option to "Centered" (which is the default already), then ensure that the rendered page has a title that is indeed center aligned.
Loading the article component styles should now also make an article intro block a slightly larger font-size than the rest of the article. This wasn't working previously.