generoi / genero-design-system

https://gds.generogrowth.com/
MIT License
4 stars 0 forks source link

feat(logo-grid): support customizing the grid through variables #30

Closed oxyc closed 4 years ago

oxyc commented 4 years ago

I hate this grid! :D Insanely complicated to create an internal border!

  1. What was used before is currently a bit broken and you can see it in the current storybook.
Screen Shot 2020-08-25 at 14 21 09

I was planning to fix it by using nth-last-child selector together with ~ selector but apparently you cannot use ~ with ::slotted. Only compound selectors and no combinators are allowed.

Before I reached that conclusion I also learned that sass doesnt support & selector within ::slotted() (makes sense). There's a proposal being worked on for it and meanwhile material design does stuff like this.

  1. Next consideration was grid-gap: 1px, where we could set a background color on the grid, and then override it on the cells. Doesn't work since empty cells will have the background color of the grid. We could fix it in the Stencil component but not in WP.

  2. Next up, I thought about adding an absolute positioned ::after element over the entire grid with touch-events: none that would have a border that overlaps the external border of the items. But there's no way of automatically setting the correct border-color of this pseudo element to match the background color. This could still be a valid solution if we cant hide the overflow. In WP it would work since we set --block-background when a block changes the background color, but it's sort of nasty.

  3. Finally I settled with negative margins and overflow: hidden. Downside is that future variations eg hover styles cant overflow. Also due to using this approach we cannot use auto-fill columns since the width of the grid container wouldnt be known, and easily become larger than the grid, thus showing the border. This is fine I think and also applies to the previous solution.

Oh and I used minmax for the grid column sizes so that if it does need to scale down, it will. Previously it was overflowing the viewport on small screens.

WP implementation using Gallery block is:

@use '~genero-design-system/src/components/gds-logo-grid' as grid;
@use '~genero-design-system/src/components/gds-logo-grid-item' as item;

.wp-block-gallery.is-style-logo-grid {
  .blocks-gallery-grid {
    @include grid.base;

    margin-left: auto;
    margin-right: auto;
  }

  @include mq($from: medium) {
    @for $i from 1 through 8 {
      &.columns-#{$i} .blocks-gallery-grid {
        --logo-grid-item-count: #{$i};
      }
    }
  }

  .blocks-gallery-item {
    @include item.base;

    // Increase specificity to override Gutenberg selector
    // Sass needs `&&&` like styled components have.
    &.blocks-gallery-item.blocks-gallery-item {
      @include grid.item;
    }

    width: auto; // override Gutenberg

    figure {
      @include item.container;
    }

    img {
      @include item.image;
    }
  }
}

Let's hope we eventually get https://github.com/w3c/csswg-drafts/issues/2748

taromorimoto commented 4 years ago

Yeah, it feels like all those host and slot styling issues are not very polished in the web component implementations. You can see this in developer tools when inspecting these web components.

Btw, @oxyc what's the reason you seem to want to use the gds-logo-grid with just the styles in WP? Could it have just worked by just using gds-logo-grid web component directly in WP? Or did I understand correctly.

oxyc commented 4 years ago

Could it have just worked by just using gds-logo-grid web component directly in WP? Or did I understand correctly.

It could have, but now we can reuse the Gallery block instead of having to create our own. At some point maybe we can have our own block but I think it's good to have all our mixins flexible enough to be able to apply in other places.

oxyc commented 4 years ago

Mostly I don't want to deal with arrays of objects with attributes in a custom maintained block :D it gets a bit messy. We can build it later