Skip to content

[Card] Expand on Click#1806

Merged
shaurya947 merged 4 commits intomui:masterfrom
codeheroics:feat-cardExpander
Oct 7, 2015
Merged

[Card] Expand on Click#1806
shaurya947 merged 4 commits intomui:masterfrom
codeheroics:feat-cardExpander

Conversation

@codeheroics
Copy link
Contributor

This is an implementation proposal for #1354 : To be able to expand card by clicking on an element such as CardHeader, rather than only being able to do it using the CardExpandable button.

This adds a new expander prop for Card child components, which triggers the expand on click.

I also updated the card documentation as to provide a demo of the proposed use.

@codeheroics
Copy link
Contributor Author

Just did a fixup on the first commit, if anyone tried the previous version I had forgotten to commit modifications and it didn't work, sorry about that.

@shaurya947
Copy link
Contributor

@hugo-agbonon I assume that the expander prop is used to determine whether or not the card can be expanded by clicking this child. Correct me if I'm wrong. If that's the case, maybe change the prop name to something more intuitive? Like canActAsExpander?

Otherwise, this looks fine to me from a high-level (yet to run the code though). @droberts84 @MichaelTaylor3d you guys have any feedback on this?

Copy link
Member

Choose a reason for hiding this comment

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

Why do we have a onClick and not a onTouchTap?

@codeheroics
Copy link
Contributor Author

I replaced the onClick events by onTouchTap events, and renamed expander to actAsExpander

@shaurya947 That's right, any direct child component of Card with the prop actAsExpander set to true will cause the card to be expanded on a click of this child.

@shaurya947
Copy link
Contributor

Sounds good @hugo-agbonon! Thanks!

shaurya947 added a commit that referenced this pull request Oct 7, 2015
@shaurya947 shaurya947 merged commit 58685dc into mui:master Oct 7, 2015
@codeheroics codeheroics deleted the feat-cardExpander branch October 13, 2015 17:32
Copy link
Contributor

Choose a reason for hiding this comment

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

@hugo-agbonon this will not work with React 0.14. Props can no longer be mutated.. see here: https://facebook.github.io/react/blog/2015/10/07/react-v0.14.html#breaking-changes

I'll try to fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shaurya947 @cgestes already wrote a fix, see #1868 / #1872,

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad, missed that. Merged now.

@zannager zannager added the scope: card Changes related to the card. label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: card Changes related to the card.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants