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

Hotfix/update subtitle selector #1614

Merged
merged 3 commits into from
Nov 29, 2023
Merged

Conversation

laurelfulford
Copy link
Contributor

@laurelfulford laurelfulford commented Nov 29, 2023

All Submissions:

Changes proposed in this Pull Request:

Edited to add: The testing steps have been updated to cover an additional fix I've added to this PR.

Subtitles:
This PR fixes a weird edge case where if you have:

  • A single post with a subtitle displaying
  • A homepage posts block on that single post

... the styles from the homepage posts block subtitle override the styles coming from the theme, and remove the space from underneath the subtitle in the post header. The issue is that the CSS selector changed in the release.

Font sizes:
It also updates the selectors for the smaller homepage post block sizes. The sizes for type scale 3 and smaller don't include the .ts-# selector in front of all elements, so that font size is getting picked up by other homepage post blocks with larger font sizes that normally use the theme's defaults.

I've updated the selectors to make sure styles for type scale 3 and lower don't "bleed" into other blocks on the page. I also tweaked the title size for type scale 4's headers -- it's the "default" size so the styles weren't set up exactly the same as the others originally, and weren't inheriting the right font size on different screen sizes.

How to test the changes in this Pull Request:

Testing the subtitle fix:

  1. Create a post and add a subtitle.
  2. Add a Homepage Posts block to the body of the post, and turn on the subtitles on the block.
  3. Publish and view on the front-end:

image

  1. Apply the PR.
  2. Confirm that the spacing in the header is changed, and also that the subtitle has no bottom margin when it appears in the block.

image

image

Testing the smaller font size fix:

  1. Add a series of homepage post blocks to a page, of all sizes -- sizes 1-4 specifically should be tested, but it wouldn't hurt to add others. You can also copy-paste the example code of all sizes below.
HPB - different sizes
<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"columns":2,"postsToShow":1,"typeScale":10,"sectionHeader":"Ts 10"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","columns":2,"postsToShow":2,"typeScale":9,"sectionHeader":"Ts 9"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","columns":2,"postsToShow":2,"typeScale":8,"sectionHeader":"Ts 8"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","typeScale":7,"sectionHeader":"Ts 7"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","typeScale":6,"sectionHeader":"Ts 6"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","typeScale":5,"sectionHeader":"Ts 5"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","sectionHeader":"Default"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","typeScale":3,"sectionHeader":"TS 3"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","typeScale":2,"sectionHeader":"TS 2"} /-->

<!-- wp:newspack-blocks/homepage-articles {"showCategory":true,"postLayout":"grid","typeScale":1,"sectionHeader":"TS 1"} /-->
  1. Compare against this test page, running an older block release. On the front-end, note that the paragraph font size on master is smaller throughout, and if make the browser window smaller, the font size on the 'default' (type scale 4) block's post titles doesn't decrease in size. In the editor, the 'default' (type scale 4) block's post titles will appear smaller than they should.
  2. (Also look for any other weirdness in case I'm missing something!).
  3. Apply the PR and run npm run build.
  4. Compare your localhost against the demo site again; confirm that the font sizes appear consistent on the front-end and in the editor.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@laurelfulford
Copy link
Contributor Author

@adekbadek I'd love to get your eyes on this before it's merged -- I ended up re-adding a CSS class removed in #1548 to fix an edge case display issue. An alternative would be to just make the CSS selector more specific (like .wpnbha .newspack-post-subtitle), but I was a little worried that could affect site-specific CSS, so I stuck with the original class.

I might be preserving something that's not great to keep in the long-run by doing this, though -- if the more specific selector would be better than what I've done here, I can come up with a plan for site-specific stuff. Just let me know if you have any questions, and I could be totally overthinking this 😅 Thanks!

Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

Thanks for catching that! I think it's fine to add this one selector.

@laurelfulford
Copy link
Contributor Author

Thanks @adekbadek!

We had one more report of something else font-size related, so I've bundled it in the same PR and am marking it as needing review again, so they can both be in the same hotfix.

I'm going to update the testing steps to cover both, then mark this as needing review again.

Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

the font size fizes work as described

The paragraphs below the titles change their size according to the size chosen in the block, both on the editor and on the front end

@laurelfulford
Copy link
Contributor Author

Thanks @leogermani! 🙌

@laurelfulford laurelfulford merged commit 0ccb973 into release Nov 29, 2023
9 checks passed
matticbot pushed a commit that referenced this pull request Nov 29, 2023
## [2.2.2](v2.2.1...v2.2.2) (2023-11-29)

### Bug Fixes

* update title, subtitle CSS selectors in homepage posts block ([#1614](#1614)) ([0ccb973](0ccb973))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants