Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Consider using DisplayAttribute.GroupName to determine SelectListItem's Group #2292

Closed
pranavkm opened this issue Mar 30, 2015 · 11 comments
Closed
Assignees
Milestone

Comments

@pranavkm
Copy link
Contributor

IHtmlHelper.GetEnumSelectList picks up DisplayAttribute.Name to determine an enum field's display name. It seems appropriate for it also pick up DispayAttribute.GroupName to group these enum values.

That said, this might have pretty narrow scope - there aren't too many scenarios I can think of where the values used to produce SelectListItem are annotated to start with.

@dougbu
Copy link
Member

dougbu commented Mar 31, 2015

low priority, I agree. but we pay attention to [Display(Name=x)], so why not [Display[GroupName=y)] as well?

@danroth27 danroth27 added 0 - Backlog up-for-grabs Members of our awesome commnity can handle this issue labels Apr 2, 2015
@danroth27 danroth27 added this to the Backlog milestone Apr 2, 2015
@heathyates
Copy link

@danroth27 @henkmollema Is this issue closed?

@henkmollema
Copy link
Contributor

@heathyates nope, my commit is just some try-out which got pushed accidentally.

@stebet
Copy link
Contributor

stebet commented Sep 25, 2015

So. I have this working pretty well on my fork. It displays groups depending on the GroupName in the DisplayAttribute, by default groups and non-groups are just displayed in the order they are reflected form the Attribute (no sorting by group name or item name by default, thought that could rather be done manually since the Html.GetEnumSelectList returns an IEnumerable). Here is a small screenshot:
capture
capture2

Anything else that needs to be added or changed before I send in a pull request?

@pranavkm
Copy link
Contributor Author

Looks good to me.

@stebet
Copy link
Contributor

stebet commented Sep 25, 2015

I'll send in the pull request so we can continue the discussion from a code perspective then :)

@dougbu
Copy link
Member

dougbu commented Sep 29, 2015

@Eilon wanna have this in Beta 8? @stebet is wrapping up #3201 as we speak.

@Eilon
Copy link
Member

Eilon commented Sep 29, 2015

Beta8 is closed. For RC this will be fine.

@dougbu dougbu modified the milestones: 6.0.0-rc1, Backlog Sep 29, 2015
@dougbu dougbu self-assigned this Sep 29, 2015
@dougbu
Copy link
Member

dougbu commented Sep 29, 2015

🆗 self-assigning since I've been working with @stebet on #3201

@dougbu dougbu removed the up-for-grabs Members of our awesome commnity can handle this issue label Sep 29, 2015
@dougbu
Copy link
Member

dougbu commented Sep 29, 2015

13af6f8

@dougbu dougbu closed this as completed Sep 29, 2015
@dougbu dougbu added 3 - Done and removed 1 - Ready labels Sep 29, 2015
@dougbu
Copy link
Member

dougbu commented Sep 29, 2015

Thanks @stebet !

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

No branches or pull requests

7 participants