Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor XY Grid cell mixins #11405

Merged
merged 31 commits into from
Oct 29, 2018
Merged

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Jul 14, 2018

Description

This pull request add new options, functions and mixins to the XY Grid and deprecate some others with the following objective:

  • Reduce the number of entry points to use the XY Grid and harmonize the interface between the XY Grid mixins.
  • Give a finer control on the CSS output
  • Add clarity about different usages of gutters generations

Changes

Update XY Grid static classes generation

  • Use new XY cell base/gutters/size mixins to generate classes instead of the xy-cell-static() mixin
  • Deprecate xy-cell-static() mixin (See migration notes below)

Update XY Grid mixins:

  • Add support for fractions in xy-cell-size() mixin
    Support values representing a fraction (6, 50%, 1 of 2 or 1/2...), treating them like shrink.

  • Use the contextual breakpoint in xy-cell-offset() and xy-cell-layout()
    Support for the current breakpoint context set by the breakpoint() mixin.

  • Add support for none value for $gutter-type in all XY Grid mixins
    Generate a gutters-less cell, instead of a gutter "without gutters generated" or "with 0-width gutters".

  • Add $output option to xy-cell() and xy-grid-layout() mixins
    Cell parts to output. You will need to generate others parts of the cell seperately, it may not work properly otherwise.

  • Deprecate $gutter-output option in favor of $output (See migration notes below)
    Note: Breaking change introduced in fix: respect $gutter-output in the -xy-cell-properties mixin #11081 has been replaced with a warning notice.

Add XY Grid functions:

  • xy-cell-base($size)
    Returns the appropriate CSS flex value for a cell base.

  • xy-cell-gutters($gutters, $breakpoint)
    Calculate the size of a cell gutters.

  • xy-cell-size-css($size, $gutters, $gutter-type, $breakpoint)
    Returns the appropriate CSS value for a cell size. Gutters-related arguments are required for cells with margin gutters (by default) as the gutter is included in the width.

  • xy-cell-offset($n, $gutters, $gutter-type, $breakpoint)
    Returns the appropriate CSS value to offset a cell.

Add XY Grid mixins:

  • xy-cell-size($size, $gutters, $gutter-type, $breakpoint, $vertical)
    Sets sizing properties for cells. Gutters-related arguments are required for cells with margin gutters (by default) as the gutter is included in the width.

  • xy-cell-gutters($size, $gutter-type, $gutter-position, $breakpoint, $vertical)
    Sets gutters properties for cells.

Add utility functions:

  • -zf-bool($val)
    Convert the given $val to a Boolean. Empty values are considered as false.

  • zf-is-fraction($value, $allow-no-denominator)
    Returns whether the given $value represents a fraction. Supports formats like 50%, 1 of 2, 1 per 2 or 1/2.

  • zf-parse-fraction($fraction)
    Parse the given $fraction to numerators and denumerators.

  • fraction-to-percentage($fraction, $denominator)
    Calculate a percentage from a given fraction.

Add utility mixins:

  • -zf-each-breakpoint-in($breakpoints, $zero-breakpoint, $media-queries)
    Iterates with @content through the given list of breakpoints $breakpoints.

Others changes:

  • Fix reversed $vertical option in xy-cell-reset() mixin.
  • Fix description for $gutter-type in xy-cell() mixin
  • Add missing semicolon in xy-grid() mixin
  • Add info about the cell parts and related mixins in the XY Grid documentation

Impact

🚚 CSS

With the disctinction it provide about different gutters generation usages, this PR remove useless gutters added in #11081 and let us saving some KB.

You can see the diff between this PR and develop at https://www.diffchecker.com/lMRwqPWa

⚠️ Deprecations

  • All XY cell mixins/functions: deprecate the $gutter-output option
    We now recommand the usage of the following options according to the needs:
    • Use $gutter-type: none for cells without gutters.
    • Use $gutters: 0 for cells with a 0-width gutters.
    • Use $output: (...) without the gutters keyword for cells with gutters that must be generated seperately.
    // Before
    @include xy-cell($gutter-output: false);

    // After
    @include xy-cell($output: (base size));
  • Deprecate the xy-cell-static() mixin
    Use the xy-cell() mixin instead. As xy-cell-static() do not generate the cell base, you may need to use the $output option, like the following.

      // Before
      @include xy-cell-static();
    
      // After
      @include xy-cell($output: (size gutters));

💥 Breaking changes:

  • All XY cell mixins/functions: a valid gutter config and breakpoint is now required.
    When no gutter can be found to generate a cell, an error is now thrown.

  • The xy-cell-offset() mixin does not support raw values for $breakpoint anymore
    To add an offset to a XY cell with custom gutters, use the $gutters option instead.

      // Before
      @include xy-cell-offset(6, $breakpoint: 16px);
    
      // After
      @include xy-cell-offset(6, $gutters: 16px);
  • The xy-cell() and xy-cell-base() mixins now generate the XY cell base CSS when $size represents a fraction.
    If you call xy-cell() with a fraction but do not want to generate the full cell, use the $output option.

      // Before
      @include xy-cell-base(shrink);
      (...)
      @include xy-cell(12);
    
      // After
      @include xy-cell-base(shrink);
      // or @include xy-cell(shrink, $output: (base));
      (...)
      @include xy-cell(12, $output: (size gutters));

    If you call xy-cell-base() dynamically and do not want to generate the base for each keyword/fraction you pass it, check $size before calling the mixin or generate the base seperately.

    // Before
    @each $size in $my-array-of-sizes {
      .my-cell.my-cell-#{$size} {
        @include xy-cell($size);
      }
    }

    // After.
    .my-cell {
      @include xy-cell-base();
    }
    @each $size in $my-array-of-sizes {
      .my-cell-#{$size} {
        @include xy-cell($size, $output: (size gutters));
      }
    }

TODO ⚠️

  • Provide a cell-oriented mixin for each part generated by xy-cell()
  • Make some tests to make sure that everything works as before
  • Update documentation

Types of changes

  • Documentation
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to change)

Checklist (all required):

  • I have read and follow the CONTRIBUTING document.
  • There are no other pull request similar to this one.
  • The pull request title is descriptive.
  • The template is fully and correctly filled.
  • The pull request targets the right branch (develop or develop-v...).
  • My commits are correctly titled and contain all relevant information.
  • My code follows the code style of this project.
  • I have updated the documentation accordingly to my changes (if relevant).
  • I have added tests to cover my changes (if relevant).
  • All new and existing tests passed.

ncoden added 6 commits July 13, 2018 23:43
Use `left right` gutters for horizontal cells and `top bottom` gutters for vertical cells by default

BREAKING CHANGE
This allow to explicitely generate a gutters-less cell, instead of a gutter "without gutters generated" or "with 0-width gutters".

This is a part of a bigger refactor of xy-cell and gutters usage,
Add the `$output` list to the xy-cell mixin for a finer control of what should be output, and an explicit indication that each part may still have to be generated seperately.

`$gutter-output` still works the same way, and breaking change introduced in foundation#11081 has been replaced with a deprecation notice. Migration notes and advices to correctly generate gutters will be included in the release notes of the future version
…asses

Deprecate the` xy-cell-static()` mixin and use `xy-cell()` mixin to generate the XY Grid static classes.

The new arguments of `xy-cells` (added in previous commits) have the following goal: making a clear distinction between cells that should not have any gutter, cells from which the gutter must be removed and cells that should have a gutter but generated seperately/later. See migration notes below.

Changes:
- Add deprecation warning in `xy-cell-static()`
- Add deprecation note in `xy-cell-static()` documentation
- Use `xy-cell()` instead of `xy-cell-static()` with the transformations listed below

Migration notes:
- Use `$output: (...)` instead of separate calls to `xy-cell-static()` and `xy-cell-base()`. This may change in the future with 3 distinguish mixins to generate "base", "size" and "gutters" of the cell.
- Use `$gutter-type: none` for cells without gutters
- Use `$gutters: 0` for cells with a 0-width gutters
- Use `$output: (...)` without "gutters" for cells with gutters that must be generated seperately.
- Remove useless `$breakpoint: $-zf-size` as it is always detected automatically
ncoden added 20 commits July 15, 2018 22:16
…y-grid-layout()`

Add support for contextual breakpoint set by the `breakpoint()` mixin to the `xy-cell-offset()` mixin, and add documentation abou existing support to the `xy-grid-layout()` mixin.
…-each-breakpoint()"

Changes:
* Add mixin `-zf-each-breakpoint-in()`
* Add function `-zf-bool()` to convert values to Booleans
* Refactor mixin `-zf-each-breakpoint()`  to rely on `-zf-each-breakpoint-in()`

There should be no change in the `-zf-each-breakpoint` API
Move all the `xy-cell()` mixin logics to dedicated functions and mixins.

Changes:
- Add function `xy-cell-gutters()`: Calculate the size of a cell gutters.
- Add function `xy-cell-size-css()`: Returns the appropriate CSS value for a cell size.
- Add mixin `xy-cell-size()`: ets sizing properties for cells.
- Add mixin `xy-cell-gutters()`: Sets gutters properties for cells.
- Refactor mixin `xy-cell()` to move all the logics to these new mixins.

All these functions/mixins now have the same interface. They do not always expect all the XY cell arguments, but they name and process the argument they expect the same way.

**BREAKING CHANGE**: When no gutter can be found to generate a cell, an error is now thrown
Changes:
- Add function `xy-cell-offset()`: Returns the appropriate CSS value to offset a cell.
- Refactor mixin `xy-cell-offset()` to use its corresponding function.

**BREAKING CHANGE**: The `$breakpoint` argument in the `xy-cell-offset()` mixin does not support raw value anymore. Use `$gutters` instead.
Changes:
- Add function `xy-cell-base()`: Returns the appropriate CSS flex value for a cell base.
- Refactor mixin `xy-cell-base()` to use its corresponding function.
zf-is-fraction(): Returns whether the given `$value` represents a fraction. Supports formats like `50%`, `1 of 2`, `1 per 2` or `1/2`.
Make the `xy-cell-base()` function and mixin support values representing a fraction (`6`, `50%`, `1 of 2` or `1/2`...) and treating them like "shrink".

**BREAKING CHANGE**: The `xy-cell()` and `xy-cell-base()` mixins now generate the XY cell base CSS when `$size` represents a fraction.

Static CSS did not changed.
…eakpoint-value()"

Use the original value if no breakpoint value can be found in `$map` in the `-zf-breakpoint-value()` mixin. With this commit, `$name` can safely be a String value without being considered as a breakpoint name.
@DanielRuf
Copy link
Contributor

Not sure how I can test the changes or what has changed in comparison to before.
Can you explain further @ncoden?

@ncoden
Copy link
Contributor Author

ncoden commented Oct 6, 2018

@DanielRuf hmm, the description of this pull request should describe everything. All changes in the API. New features, deprecations, breaking changes and new recommended usages are listed.

But to make you a quick resume:

  • Split the xy-cell mixin into multiple mixins: xy-cell-base, xy-cell-size-css, xy-cell-gutters. They take gutter-oriented arguments.
  • Replace the $gutter-output option by much safer options. See Deprecations for more infos.
  • Change some arguments defaults and behaiors in various XY mixins and functions so their all behave the same way across mixins/functions.
  • Some refactor and optimizations
  • Various bug fixes (already merged in v6.5)

There is a lot of changes so the git diff may be a bit confusing, but you can look at individual commits, I tried to keep a clear Git history.

@ncoden
Copy link
Contributor Author

ncoden commented Oct 17, 2018

@SassNinja @DanielRuf Will you have some time to review this?

Copy link
Contributor

@SassNinja SassNinja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your fraction utils!

Lets you create readable, nice code if someone requests 'special' stuff

@include xy-cell(2 of 10);

and even supports multi lang 😄

@include xy-cell(2 von 10);

@ncoden
Copy link
Contributor Author

ncoden commented Oct 26, 2018

@SassNinja Let me know when you finished your review 😉

@SassNinja
Copy link
Contributor

Let me know when you finished your review

I'm no grid power user (quite happy with the basic features 😉) but so far it looks good to me and makes sense.

So good to merge imo.

@DanielRuf
Copy link
Contributor

I'm no grid power user (quite happy with the basic features 😉) but so far it looks good to me and makes sense.

So good to merge imo.

Same here. We do not use these mixins in most projects so I can not test this thoroughly.

@ncoden
Copy link
Contributor Author

ncoden commented Oct 29, 2018

Thank you folks for the review! 👍
I am not a big user of the Sass XY Grid too, but I know the Grid, Flex Grid and XY Grid APIs very well, and how modular mixins and functions may be useful in big Sass projects.

@ncoden ncoden merged commit 4c290be into foundation:develop Oct 29, 2018
@joeworkman
Copy link
Member

This pull request has been mentioned on Foundation Open Source Community. There might be relevant details there:

https://foundation.discourse.group/t/foundation-for-sites-v6-6-0-is-here-farout/30/1

@k33n8nc
Copy link

k33n8nc commented Dec 10, 2019

Love the update :) Maybe this is a bit unrelated, but is there any documentation on the reworked XY Grid? The docs are not updated yet, the migration files showing most of the changes, not all of them. Haven't tried the local docs.

Anyone who could point me in the right the direction? Would love to dive into the mixins and see what i can do there :)

@DanielRuf
Copy link
Contributor

In general I can see them in the docs: https://get.foundation/sites/docs/xy-grid.html#xy-cell-base

@k33n8nc
Copy link

k33n8nc commented Dec 12, 2019

Must have missed, cause now i see.
Thanks! (y)

@nitrokevin
Copy link

nitrokevin commented Mar 18, 2020

can someone please give me an example of xy-cell-layout with the new changes to gutters, I don't really understand what needs to be done, for example a current block grid I'm using is:

 @include xy-grid-layout(2, '.flexible-item', true, $grid-margin-gutters, margin, left right, small, false);

@minchen085
Copy link

minchen085 commented Apr 25, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants