GSA / digitalgov.gov

Digital.gov: Better websites. Better government.
https://digital.gov
Other
219 stars 299 forks source link

Fix img shortcode #7705

Closed lcummings12 closed 2 months ago

lcummings12 commented 2 months ago

Problem Statement

The current production code does not support inline image positioning. Additionally, there is no class for centering images, resulting in inconsistent alignment and layout issues. Refactoring the image handling logic and introducing an image-center class will provide flexibility in image positioning and improve visual quality.

Summary

I solved the problem by refactoring the img.html template to correctly handle image alignment. Specifically, I introduced an align prop to allow for image-left, image-right, and image-center classes, ensuring flexible image positioning. {{- $align := .Get "align" | default "" -}}

I also added an inline prop to allow centered images to appear side by side. {{- $inline := .Get "inline" | default "false" -}} I then edited the _images.scss file to implement the class for centering the image, as well as the class for inline images.

.image-center {
  @include u-display("block");
  @include u-text-align("center");

  img {
    @include u-width("card-lg");
    @include u-maxw("full");
  }
}

.image-inline {
  @include u-display("inline-block");
  @include u-vertical-align("middle");
  @include u-text-align("center");
  @include u-margin("05");

  img {
    @include u-maxw("full");
    @include u-height("auto");
    @include u-display("inline-block");
  }
}

Additionally, I deleted the conditional that automatically assigned the image-right class if no alignment argument was passed and the image width was less than 600. Instead, I introduced an align parameter that ensures alignment classes are only applied when explicitly specified. Here the check also happens for the inline prop

<div
  class="image{{- if $align }}
    image-{{ $align }}
  {{- end }}{{- if eq $inline "true" }}image-inline{{- end }}"
>

How To Test

To test these changes, I created a markdown file to use the hugo template. I then placed various images of each of the relevant props to assure they were functioning as intended. I used assets already in the repo. Pictured is the inline, left, right, and center props.

  {{< img src="tile-4" align="center" alt="UX Summit Image" >}}
{{< img inline="true" align="center" src="tile-1" alt="UX Summit Image 1" >}}
{{< img inline="true" align="center" src="tile-2" alt="UX Summit Image 2" >}}

{{< img src="tile-5" align="left" alt="UX Summit Image" >}}

{{< img src="tile-3" align="right" alt="UX Summit Image" >}}

Example Images

Screen Shot 2024-06-17 at 8 56 03 AM
  1. create a file image-test.md in the content folder.
  2. copy the test images into it.
  3. visit http://localhost:1313/image-test/
github-actions[bot] commented 2 months ago

:mag: Preview in Federalist

lcummings12 commented 2 months ago

@RileySeaburg

nick-mon1 commented 2 months ago

@ToniBonittoGSA

When using align="left" the image has margin-top: 40px applied versus margin-top: 8px when align="right" is used. This is current code and could cause some alignment issues when both are used. Just calling this out.

{{< img src="image-1" align="left" alt="UX Summit Image 1" >}}

align left

Screenshot 2024-06-21 at 9 48 40 AM
ToniBonittoGSA commented 2 months ago

@ToniBonittoGSA

When using align="left" the image has margin-top: 40px applied versus margin-top: 8px when align="right" is used. This is current code and could cause some alignment issues when both are used. Just calling this out.

{{< img src="image-1" align="left" alt="UX Summit Image 1" >}}

align left Screenshot 2024-06-21 at 9 48 40 AM

Oh, my.. please make a Trello card in the backlog list to fix that margin, @nick-mon1

Thx!