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

Fix a bug of calling prepareStyles twice #1923

Merged
merged 1 commit into from
Oct 19, 2015
Merged

Conversation

louy
Copy link
Contributor

@louy louy commented Oct 19, 2015

Closes #1908

@@ -112,7 +112,7 @@ const DropDownIcon = React.createClass({
<div onTouchTap={this._onControlClick}>
<FontIcon
className={iconClassName}
style={this.prepareStyles(iconStyle)}>{this.props.iconLigature}</FontIcon>
style={iconStyle}>{this.props.iconLigature}</FontIcon>
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use this.mergeStyle to ensure that the auto-prefixer do his job?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

<FontIcon> (and all material components) use prepareStyles when passing styles to dom elements. So we shouldn't prefix anything passed to another component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In other words, all flipping and prefixing should only be done before passing the style object to a dom element.

Copy link
Member

Choose a reason for hiding this comment

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

But what if I want to use the iconStyle property of DropDownIcon on his own?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see why you can't. iconStyle is still passed down to the internal <div> generated by <FontIcon>

Copy link
Member

Choose a reason for hiding this comment

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

You are right, I have overlook the fact that this is not a raw html component, it's FontIcon 👍. I should get some sleep

@louy louy closed this Oct 19, 2015
@louy louy reopened this Oct 19, 2015
@louy
Copy link
Contributor Author

louy commented Oct 19, 2015

Closed & reopened to restart Travis build.

oliviertassinari added a commit that referenced this pull request Oct 19, 2015
[DropDownIcon] Fix a bug of calling prepareStyles twice
@oliviertassinari oliviertassinari merged commit c5217bc into mui:master Oct 19, 2015
@oliviertassinari
Copy link
Member

Thanks @louy!

@louy louy deleted the patch-4 branch October 19, 2015 21:24
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Nov 10, 2020
@zannager zannager added the docs Improvements or additions to the documentation label Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DropDownMenu not worked!
3 participants