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

New prop for DropDownMenu: displayMemberOnLabel #2285

Merged
merged 5 commits into from
Nov 27, 2015

Conversation

Sly777
Copy link

@Sly777 Sly777 commented Nov 26, 2015

Prop Name: displayMemberOnLabel
Default Value: null
Prop Type: String

To show different value on selected item (for example; when you want to show country names on list but country code on Label, you can use this)

@Sly777
Copy link
Author

Sly777 commented Nov 26, 2015

I added to SelectField too, @oliviertassinari

@oliviertassinari
Copy link
Member

Thanks.
I think that we should call this property labelMember to stay coherent with valueMember and displayMember.
Could you set a default value to text for the property in the getDefaultProps() method and add this property in the documentation?

@oliviertassinari oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Nov 26, 2015
@Sly777
Copy link
Author

Sly777 commented Nov 27, 2015

Hey @oliviertassinari. i updated everything as you want.

@@ -210,7 +213,7 @@ const DropDownMenu = React.createClass({

let selectedItem = this.props.menuItems[selectedIndex];
if (selectedItem) {
displayValue = selectedItem[displayMember];
displayValue = selectedItem[labelMember || displayMember];
Copy link
Member

Choose a reason for hiding this comment

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

displayValue = selectedItem[labelMember];
May be better

@oliviertassinari
Copy link
Member

@Sly777 Thanks, it's almost ready to be merged 👍

@Sly777
Copy link
Author

Sly777 commented Nov 27, 2015

@oliviertassinari i updated doc & select-field. If it's still not okay, i can restore to back ;)

@oliviertassinari
Copy link
Member

It's ok for me 😃.
Do you want to add an example in the doc?

@oliviertassinari oliviertassinari added Review: accepted and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Nov 27, 2015
@Sly777
Copy link
Author

Sly777 commented Nov 27, 2015

I added examples 👍 aaaaandd it's finished i guess 😄

@oliviertassinari
Copy link
Member

@Sly777 Nice. I would normally ask to squash down the commit, but since it's your first contribution, let's skip it.

oliviertassinari added a commit that referenced this pull request Nov 27, 2015
New prop for DropDownMenu: displayMemberOnLabel
@oliviertassinari oliviertassinari merged commit 98644a0 into mui:master Nov 27, 2015
@andrejunges
Copy link
Contributor

wouldn't be better if we set labelMember = displayMember if it wasn't passed instead of set it to 'text'?
I'd say in most of the cases we want the same property to be displayed in both places..
Besides it'll break some apps (my case)..

@oliviertassinari
Copy link
Member

Besides it'll break some apps (my case).

That's a regression indeed! Sorry.

wouldn't be better if we set labelMember = displayMember if it wasn't passed instead of set it to 'text'?

Sounds good.
Could you submit a PR for this? Otherwise can you open an issue and I will fix it.
Thanks for raising this issue!

@zannager zannager added the component: menu This is the name of the generic UI component, not the React module! label Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants