Material-components-web: MDC-Card border

[MrChriZ]

Is there any reason why the mdc-card__actions does not have a border option like the MDL-Card did - 'mdl-card--border'?

Thanks,
Chris.

[amsheehan]

Are you referring to the 1dp stroke above the action buttons here?

Nothing in particular from our side, might have been an oversight. Would you mind making a pull request with this change?

[MrChriZ]

I am yes. I've taken a look at the SCSS and wouldn't feel comfortable making that change at present - I'm kind of new on some of these front end side of things, and the MDC SCSS differs from the MDL for the card component that it doesn't look like it can be a copy/paste job. I don't have much time to fully get my head around it at present. I may revisit when things are less busy if no one has got too it before me but that maybe a month or so away.

[amsheehan]

No worries. When you would like to take a crack at it, I believe it would require three small changes:

1. In packages/mdc-card/mdc-card.scss, after the &--vertical {} block, add the following:

&--with-border {
  border-top: 1px solid rgba(0, 0, 0, .1);
}

1. In demos/card.html, copy the second demo card (the first one with actions), but add the new class mdc-card__actions--with-border to the <section> element with class mdc-card__actions. It should now look like:
   class="mdc-card__actions mdc-card__actions--with-border"

2. In packages/mdc-card/README.md document the new modifier class and explain when to use it here

I think that would be it. Let us know if you have any questions when you get around to it.

[coreymgilmore]

The mdl-card--border class was also used on mdl-card__supporting-text elements, not just mdl-card__actions.

Instead of adding the &--with-border class that would only affect the mdc-card__actions, would it not be best to just re-implement the mdl-card--border as mdc-card--border that can be added to either the mdc-card__supporting-text OR the mdc-card__actions?

Suggested:
.mdc-card--border { border-top: 1px solid rgba(0, 0, 0, 0.1); }

This could just be added to the mdc-card.scss at the end.

Aside: I would love a "configuration options" like thing for MDC like MDL has. Lists all the applicable classes nicely for a component.

[Garbee]

We'd need --border on each of those elements. Having the global modifier affect the lower children isn't proper BEM. Modifiers go onto the elements they affect directly, not children or parents of the elements in the block.

[coreymgilmore]

@Garbee So what you are saying is the organization of the classes in MDL was incorrect? Aka is is not correct per BEM?

In MDL you can do this:
<div class="mdl-card__actions mdl-card--border">...</div>

But you are saying we cannot do this in MDC:
<div class="mdc-card__actions mdc-card--border">...</div>

And we should instead do this:
<div class="mdc-card__actions mdc-card__actions--border">...</div>

If I understand you correctly, then we need two classes created:
.mdc-card__actions--border and .mdc-card__supporting-text--border

Does this make sense and fulfill requirements?

[amsheehan]

@coreymgilmore Currently we have no solution for the border on either actions or supporting text segments of cards. If a user would like to make a pull request _adding_ this support they would need to follow the instructions I outlined above, which does add a modifier to mdc-select__action i.e.: mdc-select__action--with-border (simply --border in MDL parlance).

If the request is to _also_ add a top border to the supporting text segment, you would follow the same pattern as outlined above, the resulting instructions to do so would be:

1. In packages/mdc-card/mdc-card.scss, after the &--vertical {} block (within &__actions), add the following:

&--with-border {
  border-top: 1px solid rgba(0, 0, 0, .1);
}

1. In packages/mdc-card/mdc-card.scss, within &__supporting-text, add the following:

&--with-border {
  border-top: 1px solid rgba(0, 0, 0, .1);
}

1. In demos/card.html, copy the second demo card (the first one with actions), but add the new class mdc-card__actions--with-border to the <section> element with class mdc-card__actions. It should now look like:
   class="mdc-card__actions mdc-card__actions--with-border"

2. In demos/card.html, copy the demo card with supporting text, and add the new class mdc-card__supporting-text--with-border to the <section> element with class mdc-card__supporting-text. It should now look like:
   class="mdc-card__supporting-text mdc-card__supporting-text--with-border"

3. In packages/mdc-card/README.md document the new modifier classes and explain how to use them here and here

[Garbee]

@coreymgilmore Yes, MDL was incorrect. We had quite a few things that were not correct with our BEM usage. It happens, especially when as a team we were all learning BEM while trying to implement it.

[coreymgilmore]

Working on this...

I will use mdc-card__actions--border and mdc-card__supporting-text--border since this is (1) the same as MDL, and (2) shorter.

[coreymgilmore]

PR #1645 filed.

[anton-kachurin]

@coreymgilmore @Garbee @amsheehan Please take a look at the Card Guidelines at the very bottom (search for "Dividers in cards"): https://material.io/guidelines/components/cards.html

Seems like border is getting replaced by full-bleed divider. And also inset divider was introduced.

[coreymgilmore]

@anton-kachurin So the divider we are talking about here seem to be full-bleed dividers since we are separating content blocks.

I suggest renaming the classes from mdc-card__actions--border to mdc-card__actions--full-bleed-divider. This clarifies what the divider will look like and allow for adding the inset divider in the future.

[anton-kachurin]

I would rather like to see it implemented as separate elements, and modifiers applied to those elements, i.e:

<div class="mdc-card demo-card">
  <section class="mdc-card__supporting-text">
    Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor.
  </section>
  <section class="mdc-card__divider mdc-card__divider--full-bleed"></section>
  <section class="mdc-card__actions">
    <button class="mdc-button mdc-button--compact mdc-card__action">Action 1</button>
    <button class="mdc-button mdc-button--compact mdc-card__action">Action 2</button>
  </section>
</div>

[coreymgilmore]

Why create a whole new element for a simple divider line? We are only adding styling, no content.

If we are going to use a new element, wouldn't <hr> make the most sense?

[anton-kachurin]

You're correct, <hr> would totally make more sense. And this is exactly the reason why I would go with a separate element instead of styling: divider semantically is closer to <hr> than to CSS.

[coreymgilmore]

Ok. I like this method...keeps the divider separate.

@Garbee & @amsheehan thoughts?

And how about this, to hit two birds with one stone. The full-bleed divider is accomplished by just the mdc-card__divider. Then we can modify it to inset as needed.

Styling:

&__divider {
    margin: 0;
    border-top: 1px solid rgba(0, 0, 0, .1);
    border-bottom: none;

    &--inset {
      margin-left: 16px;
      margin-right: 16px;
    }
  }

HTML would be:

<div>
    <div class="mdc-card >
    <section class="mdc-card__primary">
        ...
    </section>
    <hr class="mdc-card__divider">
    <section class="mdc-card__supporting-text">
       ...
    </section>
    <hr class="mdc-card__divider mdc-card__divider--inset">
    <section class="mdc-card__supporting-text">
       ...
    </section>
    <hr class="mdc-card__divider">
    <section class="mdc-card__actions">
        ...
    </section>
    </div>
</div>

[amsheehan]

I'm conflicted. We kind of spam the library with semantically incorrect uses of <hr> already so I hesitate to suggest moving the goal post...but we already spam the library with semantically incorrect uses of <hr> according to w3c, so maybe we should find a better way.

(coincidentally, they also moved the goal post in terms of semantic meaning of the <hr> tag)

[coreymgilmore]

What do you suggest @amsheehan?

It doesn't make sense to add either a div or section just to get a horizontal line since there isn't any content. It is just a "line" after all.

Sure this usage of hr isn't a "paragraph-level" thematic break, but it is a thematic break and it is a horizontal rule which seems close enough.

You really can't beat the hr method for ease-of-use and ease-of-readability. As far as spamming the library with incorrect uses of hr, this usage seems pretty close to semantically correct.

[Garbee]

The spec may be giving the borders a direct name, however that doesn't mean it needs to be its own element. At the core, these dividers are not having any impact on the content flow or sectioning of the nodes. Using <hr> specifically is _incorrect_ usage of that element.

Even making it a <div> or some other generic node, we need to ask ourselves why?. These dividers are purely modifications to the view of the element without impacting functionality. So why should they be independent nodes with their own place in the DOM?

My understanding of the situation which the MD spec is creating and comparing that to web architecture; leads me to believe that these dividers should only exist as modifiers on the elements. These are purely style changes in context and aren't affecting the content flow in any meaningful way. Therefore, they should exist only in CSS to create the effect.

[coreymgilmore]

So @Garbee, do you suggest going back to the original proposal?

That is, creating the following classes:
mdc-card__actions--divider and mdc-card__supporting-text--divider

And adding these classes like so:
<section class="mdc-card__supporting-text mdc-card__supporting-text--divider"></section>
<section class="mdc-card__actions mdc-card__actions--divider"></section>

If we implement in this way we should also add the mdc-card__supporting-text--divider-inset class. I don't think we need it for actions since you should only have one actions element while you could have many supporting-text elements.

I am partial to @anton-kachurin's <hr> method since it is a separate element, its clean, and we don't need duplicate modifier classes for supporting-text and actions. But I also understand @amsheehan's thoughs that <hr> is overused.

Regardless, I think we just need do decide on a solution and implement it.

[coreymgilmore]

So since it doesn't look like we can come to an agreement on implementing this I have moved on. I implemented the <hr> method.

For reference to anyone who comes along looking for a solution, I implemented the following in my custom CSS:

.mdc-card hr.mdc-card__divider {
    border-top:         1px solid rgba(0, 0, 0, .1);
    border-bottom:      none;
    margin:             0;
}

And it is implemented like so:
```

---

...

...