design-system-components icon indicating copy to clipboard operation
design-system-components copied to clipboard

Images nested under au-body class receive margin-top space by default

Open jcc-a opened this issue 5 years ago • 1 comments

Bug Report

  • [x] I’ve read and understood the Contributing guidelines and have done my best effort to follow them.
  • [x] I’ve read and agree to the Code of Conduct.
  • [x] I’ve searched for any related issues and avoided creating a duplicate issue.

What happened

img elements nested under a au-body class receive a margin-top space by default. This causes positioning issues for third-party widgets (e.g. Google Maps). When Google Maps control buttons such as zoom (-/+), fullscreen, etc are in the active state they are are incorrectly positioned. Refer to screen recording below.

What I expected to happen

Images nested under a au-body class should not receive unintentional whitespace. Images under au-responsive-media are expected to receive this whitespace.

Reproducing

  • Module name: @gov.au/body, @gov.au/responsive-media
  1. Create basic HTML page
  2. Add @gov.au/body @gov.au/responsive-media as dependencies
  3. Add resulting pancake.min.css to HTML
  4. Add simple Google Maps API script to HTML
  5. Visit HTML page and use mouse cursor to hover over Google Maps controls

Attachments

Demo of default image margin conflicting with Google Maps controls: https://drive.google.com/open?id=1DylQYhz_f5qLtgzW_juZwoS_YJ5jY4ub

Questions / Notes

Can the .au-body * + img part of the selector be removed from the responsive-media module?

https://github.com/govau/design-system-components/blob/b4dcf82337c848eddd2a63f10b5b39ec997bfb04/packages/responsive-media/src/sass/_module.scss#L58-L62

This selector was added as part of #321.

Currently this behaviour can be overridden with:

// Unset image default margin provided by responsive-media.
.au-body * + img {
	@include AU-space( margin-top, 0 );
}

Potential side effects / issues of removing this selector:

GovCMS Drupal 8 uikit theme uses au-body as a top-level page container class:

https://github.com/govCMS/govcms8_uikit_starter/blob/7eca712f8af65f87fa4ce6abd9bc5b9c7ea8ca19/templates/layout/page.html.twig#L46-L52

As does the current design-system-site for the page container main element:

https://github.com/govau/design-system-site/blob/d9608b20e98b239a325088e17419bb2a09a1c7c2/src/layout/page.js#L68

Considering the above, any images added by CMS content editor or in nested templates receive this whitespace by default. Removing this selector would result in no space between images and preceding elements — this would be visually incorrect — images and elements would appear to clash together.

To work around this, the responsive-media class might be added to any images added to templates. For Drupal, a theme override might be implemented so the the responsive-media class is added to any image output.

As a related note, Bootstrap had issues with an implicit responsive image default approach and now use an opt-in explicit class to prevent conflicts with third-party widgets. https://github.com/twbs/bootstrap/blob/275cd7f91eed9f4051d85c295e5c14ef08937804/scss/_images.scss#L1-L10

jcc-a avatar Apr 25 '19 23:04 jcc-a

Thanks for the issue @jcc-a!

As you mentioned, I don't believe this would be a straight forward fix, but it will be something that is looked at 😄

azerella avatar Apr 29 '19 01:04 azerella