Skip to content
  • Alex Ghiculescu's avatar
    49c2e518
    Allow `f.select` to be called with a single hash containing options and HTML options · 49c2e518
    Alex Ghiculescu authored
    I do this a lot:
    
    ```erb
    <%= select :post, :author, authors, required: true %>
    ```
    
    It doesn't work; the `required` attribute is ignored! Instead, you need to do this:
    
    ```erb
    <%= select :post, :author, authors, {}, required: true %>
    ```
    
    It's hard to remember the right API, and it looks to me like a code smell. It looks even smellier when you end up with this:
    
    ```erb
    <%= select :post, :author, authors, { include_blank: "Choose an option" }, { required: true } %>
    ```
    
    Where this would be nicer, but again, the `required` attribute is ignored:
    
    ```erb
    <%= select :post, :author, authors, include_blank: "Choose an option", required: true %>
    ```
    
    This PR implements a special handling for `required`, `multiple`, and `size` HTML attributes so that these now do the same thing:
    
    ```erb
    <%= select :post, :author, authors, include_blank: "Choose an option", required: true %>
    <%= select :post, :author, authors, { include_blank: "Choose an option" }, { required: true } %>
    ```
    
    ps. as proof I'm not the only person who makes this mistake, one of the tests in the Rails test suite was wrong! The test added in https://github.com/rails/rails/pull/40522 puts the `multiple` attribute in the wrong place and has the wrong assertion as as result. This PR includes a fix for the test.
    49c2e518
    Allow `f.select` to be called with a single hash containing options and HTML options
    Alex Ghiculescu authored
    I do this a lot:
    
    ```erb
    <%= select :post, :author, authors, required: true %>
    ```
    
    It doesn't work; the `required` attribute is ignored! Instead, you need to do this:
    
    ```erb
    <%= select :post, :author, authors, {}, required: true %>
    ```
    
    It's hard to remember the right API, and it looks to me like a code smell. It looks even smellier when you end up with this:
    
    ```erb
    <%= select :post, :author, authors, { include_blank: "Choose an option" }, { required: true } %>
    ```
    
    Where this would be nicer, but again, the `required` attribute is ignored:
    
    ```erb
    <%= select :post, :author, authors, include_blank: "Choose an option", required: true %>
    ```
    
    This PR implements a special handling for `required`, `multiple`, and `size` HTML attributes so that these now do the same thing:
    
    ```erb
    <%= select :post, :author, authors, include_blank: "Choose an option", required: true %>
    <%= select :post, :author, authors, { include_blank: "Choose an option" }, { required: true } %>
    ```
    
    ps. as proof I'm not the only person who makes this mistake, one of the tests in the Rails test suite was wrong! The test added in https://github.com/rails/rails/pull/40522 puts the `multiple` attribute in the wrong place and has the wrong assertion as as result. This PR includes a fix for the test.
Loading