Problem/Motivation

Columns and classes attributes in paragraphs.grid_layouts.yml are currently required because if we don't define them we will get PHP errors. But in multiple places, we have a need to just define grid layout style which is more a simple layout style and it's not actually a full grid style.

For example instead of 

design_theme_centered:
  title: 'Centered'
  description: 'Centered.'
  wrapper_classes:
    - text-center
  columns:
    -
      classes:
        -

it would be nicer that we are able to just define:

design_theme_centered:
  title: 'Centered'
  description: 'Centered.'
  wrapper_classes:
    - text-center

Proposed resolution

Improve checkings in PHP so the code can allow this.

Remaining tasks

Do the patch and discuss the proposed solution.

CommentFileSizeAuthor
#2 dont-require-columns-and-classes-2923977-2.patch900 bytespivica
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pivica created an issue. See original summary.

pivica’s picture

pivica CreditAttribution: pivica at MD Systems GmbH commented
Status: Active » Needs review
FileSize
dont-require-columns-and-classes-2923977-2.patch900 bytes

Here is a patch.

Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented
Status: Needs review » Needs work

I think it makes sense. Maybe we want to test that case. Probably it can be added to the test layouts yml file and then tested.

pivica’s picture

pivica CreditAttribution: pivica at MD Systems GmbH commented

Test make sense will add it.

I know we are stretching a definition a here a bit but from our previous discussion:

Now we are maybe abusing the system, but we are anyway doing it because we allowed empty columns attribute in the first place - so i am just saying if its empty then let's not force people to write it in yml empty definitions.
And it maybe makes sense to call this system a Layout plugin and not GridLayout because Grid is a just special case of general layout rules. But that is totally other issue and I am not saying we should rename this now, but just modify it a little bit so it can be used for more general layout rules ;)

pivica’s picture

pivica CreditAttribution: pivica at MD Systems GmbH commented

However it also makes sense to split this to Layout and GridLayout - then we allow a user to combine this two things - maybe he wants 50% centre (layout) and two columns (grid layout)... But not sure do we need that kind of flexibility or current approach is enough.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented

Yeah, definitively we need an example layout that uses this new feature. Then it's easy to use it and check if it works.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Fixed

Fine without an example for now, it's just adding a check.

  • Berdir committed 810ed8b on 8.x-1.x authored by pivica 
    Issue #2923977 by pivica: Don't require columns and classes attributes...

Status: Fixed » Closed (fixed)

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