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

Migrate some image tags #3863

Merged
merged 11 commits into from
Jun 20, 2024
Merged

Migrate some image tags #3863

merged 11 commits into from
Jun 20, 2024

Conversation

SSPJ
Copy link
Member

@SSPJ SSPJ commented Jun 7, 2024

Pull request summary

Partially does #3862.

Reminder - please do the following before assigning reviewer

  • Update readme
  • For frontend changes, ensure design review
  • For content changes beyond typos, add Ron Bronson as a reviewer

And make sure that automated checks are ok

  • fix houndci feedback
  • ensure tests pass
  • federalist builds
  • no new SNYK vulnerabilities are introduced

@SSPJ SSPJ requested a review from a team as a code owner June 7, 2024 21:25
@SSPJ SSPJ mentioned this pull request Jun 7, 2024
@@ -69,3 +69,54 @@
text-indent: units(-2);
}

.height-67px {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a reasonable temporary solution, but it's not a pattern I'd want us to establish for future blog posts with images. Let's see if we can iterate on this before merging.

Copy link
Member

@beepdotgov beepdotgov Jun 17, 2024

Choose a reason for hiding this comment

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

I've got a speculative fix for this in 3796cb7: I say "speculative", because I think the post layout isn't currently getting applied to this particular blog post. (Edit: might be related to #3849.) Let me know how you'd like me to proceed: I did a quick in-inspector test to make sure the layout works, but this will likely need visual QA/refinement once the layout's fixed. (Happy to revert the style edit, too!)

@beechnut
Copy link
Contributor

Heartburn about the width-### classes aside, this looks good to merge

@beechnut beechnut merged commit 5df142d into replatform-main Jun 20, 2024
5 checks passed
@beechnut beechnut deleted the sspj/more-image-tags branch June 20, 2024 13:58
@beepdotgov
Copy link
Member

Heartburn about the width-### classes aside

I knew I liked you, @beechnut

Thank you so much for the review, and for all the help getting this over the line!

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.

4 participants