Skip to content
This repository has been archived by the owner on Nov 30, 2020. It is now read-only.

feat(card): add primary action component #292

Merged
merged 2 commits into from
Jun 17, 2019

Conversation

tychenjiajun
Copy link
Contributor

@tychenjiajun tychenjiajun requested a review from matsp June 13, 2019 09:02
@matsp
Copy link
Owner

matsp commented Jun 17, 2019

@tychenjiajun In the case of switching from normal card to primary action card at runtime, I would have to duplicate the component and render conditionally by a prop?

@tychenjiajun
Copy link
Contributor Author

tychenjiajun commented Jun 17, 2019

@matsp No, you just need to duplicate the contents in default slot.

@matsp
Copy link
Owner

matsp commented Jun 17, 2019

@tychenjiajun Didn't see the benefit yet. The user will experience more markup. To have it all in one component is more complex for us but the user will benefit from it by writing less. Maybe I am missing something. Can you describe your idea to split it? I just want to be sure to find the right balance between development and end user experience 😅

Copy link
Owner

@matsp matsp left a comment

Choose a reason for hiding this comment

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

After reading the issue this PR make sense!

@matsp matsp merged commit 980e706 into matsp:master Jun 17, 2019
@tychenjiajun
Copy link
Contributor Author

tychenjiajun commented Jun 18, 2019

@matsp And considering accessibility, it's hard to bind a keydown event to the actionable content.
https://codepen.io/chankcccc/pen/qzNKyb
https://codepen.io/chankcccc/pen/MMeXeq

@tychenjiajun tychenjiajun deleted the card-action branch June 18, 2019 14:18
@tychenjiajun
Copy link
Contributor Author

@matsp We can also keep an actionable prop and use conditional rendering to provide a better user experience. When actionable is true, render a built-in mdc-card__primary-action and bind all the event listeners to it. In this way, users can choose either actionable prop or an independent and highly customizable <m-card-primary-action> component.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants