Problem/Motivation

Currently we have only one available grid layout that (for now) ships with the main module. We would like to have more.

Proposed resolution

Make a list of grid layouts that we want and add them to the demo module. Class naming should be as specified by https://www.drupal.org/node/2837991#comment-11885550

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Primsi created an issue. See original summary.

Primsi’s picture

Issue summary: View changes
pivica’s picture

Assigned: Unassigned » pivica
Primsi’s picture

Priority: Normal » Major
pivica’s picture

Status: Active » Needs review
FileSize
39.58 KB
99.21 KB

Here is finally a first patch for this.

Added bunch of 2 and 3 columns layout definition and appropriate CSS rules. We have basic responsive behavior in styles also but just with one break point.

Also improved demo install code and added new page that is showing just layout examples. Attaching screenshot how this is looking.

All images that we are using here are free.

miro_dietiker’s picture

Is the 1/3 image not right aligned because it is scaled down too small from the image style?

Happy to cover these problems in follow-ups, but i think they are very important:
How are we easily managing images inside a grid so they are full width?
IMHO the user expectation is that an element is covering the layout width.

Also we should discuss management / handling of paddings and card style layouts.
These change if background colors change or cards have a border. It also varies with nesting.
And sometimes if two things are combined, they need specific integration to avoid infortunate double spacing or missing spacings.

pivica’s picture

There are couple of problems here.

First we are inheriting weird CSS rules from bartik/field.css

.node .field--type-image {
    float: left;
    margin: 0 1em 0 0;
}

This float and right margin for sure will mess with our layout.
We could overwrite this but again modules should not override theme CSS code, it should be other way around.
Other solution is to not to render .field--type-image class and that field div wrappers, but again we would need to be very opinionated in paragraphs_collection module about this - not a good thing.

Second problem is that we are using very small medium image style, at least we should convert to large which is still small (480px) but better then 220px that we are getting from medium.

Finally usually there are some additional styles that are applied to images to make them more responsive in behavior. Bartik already adds most of them

img {
    max-width: 100%;
    height: auto;
}

And again if we want to change something here it should be done on theme level and not on module level.

Primsi’s picture

Status: Needs review » Needs work

This looks nice. Tested this manually and here a few initial observations:

  1. What is the difference between "Two equal columns" and "Two columns 1 - 1"
  2. We can probably remove the previous "Two column layout"
  3. For me "Tree columns 3 -1" didn't seem to work. It didn't apply the grid layout
pivica’s picture

Assigned: pivica » Unassigned
Status: Needs work » Needs review
FileSize
692 bytes
39.64 KB

> What is the difference between "Two equal columns" and "Two columns 1 - 1"

I can't find "Two equal columns" layout - from where did you get this?

> We can probably remove the previous "Two column layout"

Yeah this is defined in paragraphs_collection... i guess we can remove this right? Note that we have a test attached to this also...

> For me "Tree columns 3 -1" didn't seem to work. It didn't apply the grid layout

Good catch, fixed.

  • Primsi committed 8363ba8 on 8.x-1.x authored by pivica
    Issue #2848503 by pivica, Primsi, miro_dietiker: Add some grid layouts...
Primsi’s picture

Status: Needs review » Needs work

I can't find "Two equal columns" layout

Sorry, it's called "Equal columns". But I see that you apply different classes there, so I guess it's intentional :)

Committed. Please add follow ups mentioned in #6 if needed, then we can close the ticket.

pivica’s picture

> Sorry, it's called "Equal columns". But I see that you apply different classes there, so I guess it's intentional :)

'Equal columns' means no matter how much items you add they will all always be the same size. If naming is not clear maybe we should change the label?

> Committed. Please add follow ups mentioned in #6 if needed, then we can close the ticket.

Hmm not sure currently what follow ups to create except that we should use larger image style, here is the issue #2857765: Add bigger image style for demo purpose. For the rest of the stuff in #6 and #7, my opinion is that we should definitely not override bartik styles in contrib modules.

Related to:

Also we should discuss management / handling of paddings and card style layouts.
These change if background colors change or cards have a border. It also varies with nesting.
And sometimes if two things are combined, they need specific integration to avoid infortunate double spacing or missing spacings.

Again this is more theming stuff - theme should provide layout rules and also styles for cards, etc. We will provide some very basic rules for card, but this should be kept very minimal. Also we still do not have a card type. So not sure exactly should we create some follow up here.

Ginovski’s picture

Added followup for the two column layout removal.
#2864175: Remove the two column layout

Primsi’s picture

Status: Needs work » Fixed

If naming is not clear maybe we should change the label?

Let's see how that works out, we can change the label later. I think we can close this issue now.

Status: Fixed » Closed (fixed)

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