feat: added item-tile, item-tile-group, and layout-grid

[agliga]

Description

• Added item-tile
• Added item-tile-group
• Added layout-grid
• Modified file-preview-card to allow custom icon-button actions and href

Context

• I added a requirement for marko 5 for these new components. Since marko 4 is deprecated and adding support for it would be extra tech debt, any new components will be marko 5 or greater. (This is why we dont have a marko-tag.json)
• Formatter for marko files is broken currently. Even if we merge as is, when we do the final release, we run prettier on the whole project, so those will be fixed.
• Added a new layout section in storybook where item-tile and item-tile-group as well as layout-grid is located

Structure

The structure I ended up going with (seemed the simpliest but also offering the most flexibility) was:

<ebay-item-tile on-action("emit", "action") href="https://ebay.com"
        file={
            name: "file-name.jpg",
            type: "image/jpeg",
            src: "https://ir.ebaystatic.com/cr/v/c01/skin/docs/tb-real-square-pic.jpg",
          }
          ...input
>
    <@action aria-label="Add to watchlist">
        <ebay-heart-16-icon/>
    </@action>
    <@supertitle>
      <ebay-signal status="time-sensitive">
        Time Sensitive
      </ebay-signal>
    </@supertitle>
      <@title>
          Apple iPhone 11 Pro Max
      </@title>
      <@subtitle>
          256GB Space Gray
      </@subtitle>
    <@description>
        <div class="price">
        $29.99
        <span class="clipped"> Was: </span>
        <s class="list-price"> $68.99 </s>
      </div>
    </@description>
</ebay-item-tile>

Happy to change it around as needed, but please offer suggestions if you do not like it (and rationale)

References

eBay/evo-web#70

Screenshots

[changeset-bot]

🦋 Changeset detected

Latest commit: 76048d6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@ebay/ebayui-core	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ArtBlue]

Some things need to be adjusted, but overall looks good. I don't mind the flatter and more generic structure here. 👍

[ArtBlue]

item-tile-group is actually supposed to have predefined display rules for both gallery and list views. We should set those here and remove them from the options. Also, the <ul> inside the component needs to have a11y text, so we should add that option.

Refer to the Skin component for the predefined display rules.

[agliga]

Hmm this seems like something skin should be taking care of on its end if its predetermined
If its customizable then yeah we can default to these. But if these are fixed shouldn't these be direct styles in item-tile-group? In order to not duplicate the styles they can be referenced as mixins.

[ArtBlue]

So, we used layout-grid for this as we had planned. If we added more styling in itel-tile-group, we'd be duplicating the styles in lyout-grid. The default styling in layout-grid won't work, so we need the attributes for custom overrides. The plan all along has been to remove responsive layout styling from the components themselves and have layout-grid be the only thing that handles those. The more we duplicate the styling already present in layout-grid in other components, the less value we will see from it, and we'd needlessly increase the bundle size.

We simply need those breakpoint attributes set on .layout-grid to achieve that. One of the reasons why originally I had a separate distinct level in item-tile-group was to make this a bit clearer.

[agliga]

So I agree with that, but shouldn't those attributes just be set by item-tile-group? layout-grid handles all the layout part, but item-tile-group simply defines what happens at what breakpoint.

It could simply be:

.item-tile-group {
    --layout-grid-columns-min: 1;
    --layout-grid-columns-xs: 2;
    // etc
}

And then it should work automatically correct?

[ArtBlue]

Oh, I see. Yea, we can do that too. I wish we'd done the Skin part like that. As it stands now though, if we went with this option, there will need to be sweeping changes in Skin and this PR will be blocked until that's done. I'm fine if that's what we do, but will take longer.

[ArtBlue]

Per my previous comment, we can remove this. We just need to impose the layout rules directly in the markup on our own side.

[ArtBlue]

The various subgroups in this section need to use <p> instead of <div> because Skin has built in relative line spacing DS wanted. The current spacing is incorrect.

It may be a good idea to actually use <@attribute> to do the <p> wrapping on our end so teams can't use something that will result in spacing issues. There's a possibility for any host of attributes. That will probably also help business objects components so more brand-specific attributes continue to retain that spacing as well.

[agliga]

Sounds good. Will adjust

[agliga]

One comment, we should adjust skin since some examples have <div>

[ArtBlue]

It looks to be mostly there. It seems to still be missing the a11y label. See Skin.

[ArtBlue]

Looks good.

[ArtBlue]

It looks to be mostly there. It seems to still be missing the a11y label.

Ok, we spoke offline about this. Looks good. Approved the PR.