frankbaele is going to sprint on this tomorrow.

background info #1693036: Responsive layouts & responsive images: both need breakpoints

layout is now using only px/em to build a media query (MQ) using min/max-width
The goal is to use the breakpoints module so people can use full MQ
The breakpoints module allows you to create arbitrary groups of breakpoints

The interface needs to be changed so it works similar to that of resp_img 7.x-2.x

What needs to be done:
Change the schema, so you can map a breakpoint to a grid, see http://drupalcode.org/project/resp_img.git/blob/refs/heads/7.x-2.x:/resp...
Basically you'll store the group and a blob containing the mapping of each breakpoint to a grid, optionally add an extra width_in_pixel so the layout designer knows how wide it will be

Change the module code for loading/saving breakpoints see http://drupalcode.org/project/layout.git/blob/refs/heads/7.x-1.x:/layout...
Remove layout_breakpoint_sort_by_width because it's handled by breakpoints
Change layout_breakpoint_get_css to it uses the full MQ in the output

Change http://drupalcode.org/project/layout.git/blob/refs/heads/7.x-1.x:/plugin...

Change http://drupalcode.org/project/layout.git/blob/refs/heads/7.x-1.x:/plugin...
Use the 'fake width' to handle the builder or try to determine the width

CommentFileSizeAuthor
#7 breakpoints-integration.patch12.75 KBgábor hojtsy
#3 i1746692.patch26.06 KBattiks

Comments

jessebeach’s picture

jessebeach commented

Great, I'm looking forward to getting this module integrated into the distro.

attiks’s picture

attiks commented

I just got an email from frank that he's leaving today, so I'll do it myself tomorrow

attiks’s picture

attiks commented
Status: Active » Needs review
StatusFileSize
new i1746692.patch26.06 KB

First patch to get you started ;-)

Some remarks:

  • no upgrade functions, there's 1 db table removed and 1 added
  • no more default breakpoints, so create those first
  • if you create a panel use the same machine_name to the breakpoint group name
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Assigned: Unassigned » gábor hojtsy
Issue tags: +Spark sprint 4

Gábor is going to take a look at this for this sprint.

attiks’s picture

attiks commented

Let me know is I can help as well, the code is still a bit rough

@Gabor ping me on irc or send me an email, if I can I'll try to be online while you guys/girls are sprinting.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Looked at the patch itself and talked to @attiks over IRC. Some notes:

- the UX of the breakpoints module is pretty confusing at places, etc. you need to create a breakpoint before you create a group, then you cannot add a breakpoint to a group by adding a new one (or on the group page), you need to hit a slightly hidden "edit group breakpoints" link at the bottom to add more
- until you set up a group with at least a breakpoint, the provided patch does not display anything on the UI
- the patch itself has lots of unrelated copy-paste, eg. the entry point for layout editing is called "Responsive images and styles" - it is just a rough patch in other words
- once you provided at least a group, the layout screen displays a link for each group, clicking there lets you assign grids and approximate px values to each breakpoint (these are stored as mapping with layout module)
- in general I do not understand the need for lots of new custom code in the patch vs. our heavier reliance on ctools earlier, where we just depended on ctools doing the same things, so we needed less code

1. Overall our easiest path for this integration would be to define one breakpoint group for layouts, and just add more info on them. I'm not sure of the use cases of supporting more breakpoint groups in layout module, having seen no real life sets of breakpoints to understand the significance. This is at least a deficiency on my part.

2. Ideally we'd not need our own admin pages / storage but breakpoints module would support additional data stored with breakpoints (like views supports plugins adding more features to a view). Not sure of the implementation complexity of that for breakpoints module, but it would follow a common pattern with ctools based modules. This way of storing tightly related additional data outside the breakpoint seems weird. We already have possible consistency problems with grids, regions and layouts being different objects but relying on each other. Now we break down breakpoints to three more objects: the breakpoint, the breakpoint group and the mapping between breakpoints and grids/approximate px. So we'd have 6 objects to juggle instead of 4. If breakpoints could handle extended data within themselves, that would at least reduce this to 5, and then we'd only need to work on refining the UX of the breakpoints module.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work
StatusFileSize
new breakpoints-integration.patch12.75 KB

Worked out this initial version as the first working implementation. The breakpoints are now moved to the breakpoint module, they get a 'layout_' prefix (I assume machine names would need to be unique for breakpoints across the whole site?), but that prefix is then removed for the internal dealings with layout module. The breakpoint name and admin label are derived from that and the width is derived from the min-width in the media query.

The grid associations, which is now only machine name to machine name mappings are just stored in a variable. I see this is not ideal for exportability. A full-on ctools approach seems a bit overkill for this however. Not sure how best to work this out for everyone's happiness.

If there is no min-width, or there is no grid association, the breakpoint will not show up for the layout builder, however any breakpoint defined by the layout module will. It might actually be better to define a breakpoint group instead and rely on that? Attiks, can you help with that?