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

Replace Message with Alert, use in Comment #1373

Merged
merged 7 commits into from
Jul 9, 2021

Conversation

tylersticka
Copy link
Member

@tylersticka tylersticka commented Jul 8, 2021

Overview

The Comment component introduced in #1370 lacked a message for communicating when a comment had been submitted but was still in moderation. The live version of this feature used a component we called Message, which had been recreated in #604. But this pattern's size made it appear more unwieldy in that context, and it didn't seem to respond to our themes. I also found its name somewhat confusing: A comment is a type of message, after all, so it felt a bit off to say "display a message inside of a comment."

So this PR removes Message, introduces a new Alert pattern, and integrates that into Comment for the moderation message feature.

Screenshots

Screenshot 2021-07-08 at 15-38-10 Storybook

Screen Shot 2021-07-08 at 3 38 23 PM

Screen Shot 2021-07-08 at 3 44 11 PM

Testing

On the deploy preview…


@changeset-bot
Copy link

changeset-bot bot commented Jul 8, 2021

🦋 Changeset detected

Latest commit: ced0f4f

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 Major

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

@tylersticka tylersticka marked this pull request as ready for review July 8, 2021 23:09
@tylersticka tylersticka requested review from a team July 8, 2021 23:09
Copy link
Member

@Paul-Hebert Paul-Hebert left a comment

Choose a reason for hiding this comment

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

This looks good!

My one question is whether the alert component should support HTML being passed in instead of text? (So we could include a link, icon, etc.)

If we want to support that use case we may want to replace the message property with a content slot. If we don't want to support that use case then this looks good to go!

@tylersticka
Copy link
Member Author

My one question is whether the alert component should support HTML being passed in instead of text? (So we could include a link, icon, etc.)

If we want to support that use case we may want to replace the message property with a content slot. If we don't want to support that use case then this looks good to go!

@Paul-Hebert Either is supported!

<div class="c-alert__content">
{% block content %}
<p>
{{message|default('Hello world!')}}
</p>
{% endblock %}
</div>

@Paul-Hebert
Copy link
Member

Oh, got it. I'd missed that. Awesome! Looks good!

@tylersticka tylersticka merged commit 76051bb into v-next Jul 9, 2021
@tylersticka tylersticka deleted the feature/message-tweaks-for-comments branch July 9, 2021 16:25
@github-actions github-actions bot mentioned this pull request Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants