-
Notifications
You must be signed in to change notification settings - Fork 22k
Add weekday_options_for_select method
#42979
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
Conversation
kaspth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Some more comments and then squash the commits down to 1 and we're good to go.
bbeeb1f to
2633211
Compare
Add `weekday_select` method Create `Tags::WeekdaySelect` class Add `weekday_select` to `FromBuilder` Add Documentation Allow `WeekdaySelect` to use selected option if value is nil Doc fix Add tests Use kwrd args Update CHANGELOG Fix `Tags::WeekdaySelect` for updated kwrd args Update CHANGELOG format Condense `weekday_options_for_select` method Update tests for kwargs
2633211 to
592570f
Compare
|
Sweet, thanks! |
|
Hi, very handy PR. I think it’s more of a US thing to have Sunday start of week, could we extend this to allow Monday to be the first index? |
Since this leverages the arrays in ie: en:
date:
day_names: [Monday, Tuesday, Wednesday, Thursday, Friday, Saturday, Sunday]
abbr_day_names: [Mon, Tue, Wed, Thu, Fri, Sat, Sun] |
|
This would work for if all users where from same region, but in our application for example we have users globally, so formats are dynamic based on regional settings |
Ah, I see what you're saying. Actually, I don't think my suggestion would have worked anyway because in Ruby "Sunday" is always at 0... |
|
Ahh yep that too :) |
|
@acetinick What do you think about this? We can add another option to |
|
Hi @DRBragg yep I think that would work great, and will cover other edge case scenarios. Default of Sunday makes sense :) |
|
@acetinick how's this #43037 work for you? |
|
Is there a way to select multiple values with this? If using select2 or other similar plugins? I tried multiple: true but it doesn't seem to be working. |
Summary
In a majority of the Rails apps I've built I've had to add my own helpers for a weekday select. Given how great Rails is at providing most of the helpers you could ever want, this omission has always been a little surprising to me. Since I just had to work on this again, and another dev shared my surprise at the omission, I thought it was time for a Rails PR.
This PR adds 2 new
FormOptionHelpermethods, aFormBuildermethod, and a newTags::WeekdaySelectclass.In addition to accepting a
selectedvalue,weekday_options_for_selectcan have 2 options,:index_as_valueand/or:day_format.weekday_options_for_selectis really just a helper method for:weekday_selectis also used in theFormBuilderso can use it like this:to produce the following HTML: