Skip to content

Speaker pattern - #1607

Merged
gerardo-rodriguez merged 27 commits into
v-nextfrom
feature/speaker
Dec 3, 2021
Merged

Speaker pattern#1607
gerardo-rodriguez merged 27 commits into
v-nextfrom
feature/speaker

Conversation

@gerardo-rodriguez

@gerardo-rodriguezgerardo-rodriguez commented Nov 30, 2021

Copy link
Copy Markdown
Member

Overview

This PR refactors the Author component so that it can be used for both article and presentation bylines.

Screenshots

screencapture-localhost-6006-2021-12-03-10_18_33

Testing

  1. Review the refactored Author pattern
  2. Also review with VoiceOver to ensure the date isn't repeated twice

@changeset-bot

changeset-botBot commented Nov 30, 2021

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4a38794

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

This PR includes changesets to release 1 package
NameType
@cloudfour/patternsMinor

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

@gerardo-rodriguezgerardo-rodriguez self-assigned this Nov 30, 2021
@netlify

netlifyBot commented Nov 30, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for cloudfour-patterns ready!

🔨 Explore the source changes: 4a38794

🔍 Inspect the deploy log: https://app.netlify.com/sites/cloudfour-patterns/deploys/61aa5cf0ded7c1000847e9ee

😎 Browse the preview: https://deploy-preview-1607--cloudfour-patterns.netlify.app

@gerardo-rodriguez
gerardo-rodriguez requested review from a teamDecember 1, 2021 18:55
@gerardo-rodriguez
gerardo-rodriguez marked this pull request as ready for review December 1, 2021 18:55

@tylerstickatylersticka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@gerardo-rodriguez It seems strange to have a pattern that so closely matches the existing Author component.

Did you consider updating the Author component instead, replacing date with a more flexible/generic option?

@gerardo-rodriguez

gerardo-rodriguez commented Dec 1, 2021

Copy link
Copy Markdown
MemberAuthor

@tylersticka Hmm...good question. They are very similar.

I can take a closer look! 🔍 👀

Comment threadsrc/components/author/author.twig Outdated
@gerardo-rodriguez
gerardo-rodriguez requested review from a team and tylerstickaDecember 2, 2021 20:51
@gerardo-rodriguez

Copy link
Copy Markdown
MemberAuthor

@tylersticka This PR is ready for another review, appreciate your feedback, thanks! 🙂

@tylerstickatylersticka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for continuing to revise this, @gerardo-rodriguez!

Comment threadsrc/components/author/author.stories.mdx Outdated
Comment threadsrc/components/author/author.twig Outdated
@gerardo-rodriguez

Copy link
Copy Markdown
MemberAuthor

Thanks again, @tylersticka! This PR is ready for another review. 🎉

Comment threadsrc/components/author/author.twig Outdated

@tylerstickatylersticka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This version's awesome, @gerardo-rodriguez! More features but it actually seems simpler to use when you read the docs, which is the ideal. My comments are really just affirming some of your instincts regarding accessibility prefix options.

Comment threadsrc/components/author/author.twig Outdated
Comment threadsrc/components/author/author.twig Outdated
Comment threadsrc/components/author/author.twig Outdated
Sign up for freeto 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.

Speaker pattern

2 participants

@gerardo-rodriguez@tylersticka