Follow-up to #2334387: UI changes to support current responsive image standards

Problem/Motivation

Responsive images module now allows an empty breakpoint, but since there's no UI for the moment, you can only define this in theme.breakpoints.yml, so make it easier for people to discover this, Bartik should do the same.

For mobile first design the following are equivalent:

  • mediaQuery: '(min-width: 0px)'
  • mediaQuery: ''

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#1 i2424587-empty-breakpoint.patch365 bytesattiks

Comments

attiks’s picture

attiks commented
Status: Active » Needs review
StatusFileSize
new i2424587-empty-breakpoint.patch365 bytes
rainbowarray’s picture

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented

+1 to this change. This will make it much easier for somebody to use sizes once there is a UI for that.

Are there any tests that need to be modified due to this change?

attiks’s picture

attiks commented

No test changes needed

idebr’s picture

idebr commented
Issue tags: +Bartik
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Component: responsive_image.module » Bartik theme
Status: Needs review » Needs work
Issue tags: -Bartik +CSS

Then we should update Bartik's CSS accordingly?

attiks’s picture

attiks commented

There's nothing to update, the mobile breakpoint is never used inside the css, the reason it exists inside bartik.breakpoints.yml is to be able to use it for responsive images

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
Issue tags: -CSS

Alright :) Even better. Sorry for the mistake!

(I'm not at my computer, couldn't quickly check.)

rainbowarray’s picture

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented
Status: Needs review » Needs work

I just tested this on simplytest.me. I set up a responsive image style using thumbnail for the mobile breakpoint. When I set article image to use the responsive image style and then created an article with a test image, the source element for the mobile breakpoint had an empty media attribute.

It seems like if the media attribute is empty, then it shouldn't be added to the source element.

attiks’s picture

attiks commented

#8 New issue created #2424727: Do not output empty media attribute for source tags

attiks’s picture

attiks commented
Component: Bartik theme » responsive_image.module
attiks’s picture

attiks commented
Component: responsive_image.module » Bartik theme
rainbowarray’s picture

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented
Status: Needs work » Reviewed & tested by the community

Now that #2424727: Do not output empty media attribute for source tags is in, I tested this again, and this works well. Nice simple change.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 09dbe27 and pushed to 8.0.x. Thanks!

  • alexpott committed 09dbe27 on 8.0.x 
    Issue #2424587 by attiks: Make the mobile breakpoint for Bartik empty

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.