Switch Comment Badging to a Slot

[Paul-Hebert]

Overview

This PR makes a change to how author/member badges can be applied to comments.

Instead of using properties for this, the template now accepts an author_meta slot.

This allows the consuming app to use whatever logic makes sense to determine what badge to apply.

Screenshots

(No visual changes)

Testing

1. Review the preview deploy docs page

---

• Fixes Finalize Comment Template Properties #1429

[changeset-bot]

🦋 Changeset detected

Latest commit: 64bbd18

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

This PR includes changesets to release 1 package

Name	Type
@cloudfour/patterns	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

[Paul-Hebert]

This is technically major, but the breaking change affects a feature we're not using anywhere yet?

[Paul-Hebert]

@tylersticka I know there was some discussion of whether the entire heading content should be a block, or if just the meta info should be a block. I thought it made more sense to only affect the meta info, but I could easily be convinced otherwise.

[tylersticka]

@Paul-Hebert I think I'd like to convince you otherwise. 🙂

The approach you've taken here means a developer can append content to the heading, and that's it. They can't modify the heading content, they can't prepend content, etc. without creating additional blocks. And because there's no <div class="author-meta"> or anything wrapping it, we haven't saved them the trouble of rewriting markup. It's just kind of a very specific block that goes unused by default.

If you instead did the following:

{%block heading %}
  {{comment.author.name}}
{% endblock %}

Now we have a block with a very defined relationship to its containing element, and developers have the option of replacing it entirely, prepending content, or appending content, all without the need to create additional blocks.

I feel strongly that this would be a better approach.

[Paul-Hebert]

Sure, I've been convinced 🙂

I can switch that over. (Currently looking into some biz-dev stuff. Will circle back before EOD)

[Paul-Hebert]

@tylersticka , following up on this. Here's the current heading structure (without the meta stuff)

<h{{_heading_depth}} class="c-comment__heading">
  {{comment.author.name}}

  <span class="u-hidden-visually">
    {% if comment.is_child %}replied{% else %}said{% endif %}:
  </span>
</h{{_heading_depth}}>

I think the block should be inside the h{{_heading_depth}} tag, so perhaps it should be called heading_content?

<h{{_heading_depth}} class="c-comment__heading">
  {% block heading_content %}
    {{comment.author.name}}
  {% endblock %}

  <span class="u-hidden-visually">
    {% if comment.is_child %}replied{% else %}said{% endif %}:
  </span>
</h{{_heading_depth}}>

But this block still isn't the full heading content, since there's the visually hidden text. The visually hidden text is following some special logic right now, and it doesn't seem like the component consumer should need to recreate that. But, whatever they put in our new block needs to work with the visually hidden text following it. Maybe the block should be called author_title or something along those lines?

<h{{_heading_depth}} class="c-comment__heading">
  {% block author_title %}
    {{comment.author.name}}
  {% endblock %}

  <span class="u-hidden-visually">
    {% if comment.is_child %}replied{% else %}said{% endif %}:
  </span>
</h{{_heading_depth}}>

I went back and forth between title and name, since the block kind of does both. What do you think?

[tylersticka]

I don't think it should be called author_title because that dictates what the content is. I think heading_content would be fine.

[Paul-Hebert]

Yea, that's fair. Names are hard!

[Paul-Hebert]

@tylersticka how do you feel about these blocks?

<header class="c-comment__header">
  {% block header_content %}
    <h{{_heading_depth}} class="c-comment__heading">
      {% block heading_content %}
        {% block author_title %}
          {{comment.author.name}}
        {% endblock %}

        <span class="u-hidden-visually">
          {% if comment.is_child %}replied{% else %}said{% endif %}:
        </span>
      {% endblock %}
    </h{{_heading_depth}}>
  {% endblock %}
</header>

For our existing use case (passing in a badge) you would pass in an author_title. If you wanted to replace all of the heading content, you could do so with heading_content. If you want to replace the heading, you could pass in header_content.

(header_content may be premature?)

If this still feels off maybe we can find some time to chat with voices?

[tylersticka]

Sure, that's fine with me

[tylersticka]

(the suggested blocks, not the proposal to chat with voices... I mean, I'm fine chatting with voices, I just don't know that I need to for this!)

[Paul-Hebert]

Cool, thanks Tyler! Implementing this now

[Paul-Hebert]

@tylersticka I adjusted the blocks based on our discussion, added a replies block, and added documentation around these blocks. Let me know what you think. If possible I'd love to try integrating this into Wordpress before I leave for vacation to confirm it all works as expected.

[tylersticka]

It looks like maybe the documentation needs updating to match the new block names?

[Paul-Hebert]

Good catch, done!