Feat: Implement grid with 4 columns on medium

[jmuzina]

Done

Implements the 4-column grid on medium, with .row--4-cols.

.row--4-cols is a grid row that shares most of the behavior of .row, but has 4 columns on medium screens, instead of 6.

Fixes WD-14972

QA

• Review all 4-column-medium examples. Be sure to verify that they look correct on all breakpoints.
  • 4 columns on medium
  • 4 columns on medium, mixed responsive
  • 4 columns on medium, with offsets
  • 4 columns on medium, with reordering
  • 4 columns on medium, with nesting
  • Comparison of 4-columns on medium and 6-columns on medium
• Review 4 col medium grid docs
• Review 4.17.0 release notes

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

• PR should have one of the following labels to automatically categorise it in release notes:
  • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
• Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
  • if CSS class names are not changed it can be bugfix release (x.x.X)
  • if CSS class names are changed/added/removed it should be minor version (x.X.0)
  • see the wiki for more details
• Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.

Screenshots

[webteam-app]

Demo

Jenkins

demos.haus

[lyubomir-popov]

looks great to me. questions around naming:

• what is the shortest name we can come up with? .vf-row maybe?
• once inside a grid-row, do we need separate col classnames? maybe the original ones can be reused as the context is set by the row

[jmuzina]

Implementation for sharing the col- class across both grids took a bit longer than I thought it would - will clean this up more tomorrow to hopefully get it out of draft stage

There are a couple of placeholder/variable naming choices that were temporary that I'll revisit, and I'll make a general pass for completion as well.

[bartaz]

Couple naming suggestions in inline comments. But the overall approach is solid I think, thanks!

[pastelcyborg]

A couple high-level comments:

1. If it weren't for the code sample for the new 4-column variant, I would find the documentation/guidance on grids now quite confusing. I'm wondering if there's some way to more clearly indicate that there are essentially 2 top-level grids now, and that the 4-column variant is newer and recommended? It almost makes me think we should consider adding a deprecation notice to the old grid...
2. There's only one example using the new 4-column variant - it might be nice to include some more examples in the docs themselves.

[jmuzina]

@pastelcyborg

1. If it weren't for the code sample for the new 4-column variant, I would find the documentation/guidance on grids now quite confusing. I'm wondering if there's some way to more clearly indicate that there are essentially 2 top-level grids now, and that the 4-column variant is newer and recommended? It almost makes me think we should consider adding a deprecation notice to the old grid...

I have updated the structure table to show the difference in the two different grid classes, maybe this helps some:

In terms of stating that the 4-column variant is newer and recommended, I will end up using your "New" label from #5353 for sure. Also, we won't really be "deprecating" .row per-se in v5 - my understanding is that we will switch .row over to have 4 columns on medium (basically, .row--4-cols is a preview of the new .row). We just can't switch over the functionality in a minor version as it's breaking.

Maybe we could add some language there along the lines of "4 columns on medium will become the default behavior of .row in Vanilla v5. Please use .row--4-cols to build layouts with multiples of 4 columns on all breakpoints to avoid unexpected layout disruptions when v5 released."?

2. There's only one example using the new 4-column variant - it might be nice to include some more examples in the docs themselves.

Not sure if I understand you fully here - I have made 6 examples that use the 4-column variant. See the PR description.

Do you mean that we should embed more of them in the docs page?

[bartaz]

As discussed in standup, let's document it as a new default.

This means:

• update the grid documentation accordingly, mark .row as legacy/deprecated and not recommended to be used on new pages
• update Vanilla examples where relevant to feature new row-4-cols-medium by default
• update Vanilla website to use new row where relevant

(I'm not sure if we need to do it all in one PR, but I guess would be good to make them part of the same release)

[jmuzina]

@bartaz I've updated the PR as requested, have a look and let me know what you think.

[bartaz]

Now that we made the new row--4-col-medium a de-facto new default I kinda dislike how verbose the name is. I can't imagine how annoying it will be to use it all the time instead of row. We need to simplify it I think.

But I can't see a very good alternative:

• p-row
• grid-row
• row--new, row--next
• row--4, row--4c, row--4-col
• …

[bartaz]

If that's the new default we should list it first. And maybe add deprecated red status label to the cell listing .row

[jmuzina]

This afternoon I did some exploration of updating the common grid patterns to use 4 cols on medium, as discussed in review today.

Attempting to change the behavior of the common grid patterns (grid shorthands) to use 4 columns on medium is proving to be much more difficult than I thought it would be. Updating the base case (no nested grid) is trivial, but handling grid nesting inside these shorthands is difficult to do as a shorthand needs to be able to handle the existing .row (which sets itself to use 6 cols on medium) and the new .p-row.

Essentially this creates a case where .row sometimes has up to 6 cols on medium (if it is not nested), and sometimes has up to 4 cols on medium (when it is nested inside a shorthand grid). This creates a lot of extra complication and is very brittle.

I suspect it may be easier (if messier) to introduce separate .p- prefixed shorthand classes (and expect people to update their nested rows inside these shorthands to also use .p-row).

I have updated this PR to change the class of the 4 col grid to p-row as discussed, and updated the documentation to more explicitly state that the old .row is to be deprecated.

@bartaz @pastelcyborg

[bartaz]

Essentially this creates a case where .row sometimes has up to 6 cols on medium (if it is not nested), and sometimes has up to 4 cols on medium (when it is nested inside a shorthand grid). This creates a lot of extra complication and is very brittle.

I'm getting to the point where I feel we should not do it in current codebase.

If this issue would be to go through out new triaging process it would not land in v4 due to high effort and compatibility issues.

Having these 2 grids working side by side would be extremely confusing and problematic for devs.

I see 2 options:

• update existing grid row class to be 4 columns and deal with any breaking changes (find them and fix them if needed)
• don't do it in current codebase and leave it for new architecture

There is another possible approach: not create new class names for 4 col grid, but allow setting it on higher level:

• make it a SCSS setting - you can either combile Vanilla with 6 col, or 4 col grid - but that's obvioulsy not really migration-friendly
• add some kind of top level class (on body? or some container) has-4-col-grid (or whatever we end up calling it) that switches all grid on given page to new 4 col grid system, so you can still use good ol row as before

[pastelcyborg]

Ditto @bartaz. All plausible solutions we've considered create both confusing code and naming, which really reduce the overall value of this feature. If it's an absolute necessity for v4, concur with @bartaz, the existing grid should be modified to become 4-column and all uses of it should be fixed. If it's not an absolute necessity, I think it makes a lot more sense to thorough spec out the new grid for v5, ensure it hits all needed requirements, and build it at that time.

[bartaz]

To summarise recent decisions made:

• we don't want to postpone the new grid to new architecture
• we want to separate the implementation of the new grid and the old one, for clearer separation
  • this means having a copy of _patterns_grid.scss file for current and new grid
  • the current/old grid will use hardcoded values for column counts (4/6/12)
  • new grid will use values from grid settings that we will set to 4/4/8 (notice both medium and large screen grid changes!)
  • we need to take care of %vf-row and likely duplicate it as well
• new grid needs to use new class names for both row and columns (grid-row, grid-col- - that's the initial suggestion, Josh suggested using something more semantic if possible)
• we currently don't need to implement shorthands (like --50-50 etc) for the new grid
• existing components that use grid don't need to be updated right away, we can tackle them once new grid is out
• from design guidelines perspective
  • it's OK to have sections with different grids on the page (one section using new grid, other section using old row or row--50-50
  • it's NOT OK to mix grids within one section, especially nesting one in another
    • we may actively make it break or have some debug red colour borders if devs try to do it

[jmuzina]

Closing as not needed - the new grid will be built from scratch, using the decisions summarized here. Targeting two days for implementation.