Follow-up to [rails/rails#43411][] (merged in [15f6113][])
By default, when generating a `<button>` element through a Form Builder
instance, the element's `[name]` attribute is populated by calling the
`FormBuilder#field_name` method. This commit assigns a matching default
`[id]` attribute generated by `FormBuilder#field_id`.
Additionally, it adds test coverage to ensure that calls that provide
their own `name:` and `id:` options are not overridden by the default
values.
[rails/rails#43411]: https://github.com/rails/rails/pull/43411
[15f6113]: 15f6113622
The background
---
Configuration for replacing a collection was introduced in
[rails/rails#36716][].
However, since [rails/rails#42596][] has been merged, Rails 7.1 and
beyond will default to _replacing_ an Active Storage `has_many_attached`
relationship, as opposed to _appending to it_.
The problem
---
With replacement as the established precedent, it's currently a
challenge to replace an existing collection with an empty one.
The solution
---
This commit makes two changes.
The first is to Action View and its form building helpers. The change
draws inspiration from how an `<input type="checkbox">` field (or
collection of fields) is paired with an `<input type="hidden">` field to
represent the unchecked value. The change pairs any `<input type="file"
multiple="multiple">` elements with an `<input type="hidden">` element
to represent an empty collection. Like the [check_box][] form builder
method, the `file_field` method accepts an `include_hidden:` option to
skip the creation of the hidden element.
The second is to how Active Storage generates attribute assignment
methods through `has_many_attached`. With the possibility of an `<input
type="file">` field being paired with an `<input type="hidden"
value="">` field, the backing models need to be able to coerce an
"empty-ish" value into an empty list. For example:
```ruby
@user.highlights = [""]
@user.highlights # => []
```
When combined, these changes enable consumer applications to submit
"empty" collections to blank out existing attachments.
Support is configured through the
`config.active_storage.multiple_file_field_include_hidden` configuration
value, which defaults to `false`.
[check_box]: https://edgeapi.rubyonrails.org/classes/ActionView/Helpers/FormBuilder.html#method-i-check_box
[rails/rails#36716]: https://github.com/rails/rails/pull/36716
[rails/rails#42596]: https://github.com/rails/rails/pull/42596
When constructing the field's `[id]` attribute, the current
`FormBuilder#field_id` implementation (introduced in [59ca21c][])
ignores the `namespace:` option.
This commit incorporates any namespace by prepending it to the
`@object_name`.
[59ca21c]: 59ca21c011
Re-use template.field_id
---
Thread options[:namespace] down through the FormBuilder instance to the
`Tags::Base#tag_id` and `#add_default_name_and_id` methods
Squashed commits:
[49cb03a3ce] Fix missing return from ActionView::Helpers::NumberHelper#parse_float, fixes#43853
Add test case for number helpers not raising exception when `raise: true` is passed and input is valid
Ruby 3.1 introduced an optimization to string interpolation for some
core classes in b08dacfea3.
But since we override `to_s` in some of those core classes to add behavior
like `to_s(:db)`, all Rails applications will not be able to take advantage
of that improvement.
Since we can use the `to_formatted_s` alias for the Rails specific behavior
it is best for us to deprecate the `to_s` core extension and allow Rails
applications to get the proformace improvement.
This commit starts removing all the `to_s(:db)` calls inside the framework
so we can deprecate the core extension in the next commit.
This can save a significant amount of string allocation in some scenarios
and is more consistent with modern Ruby code where `frozen_string_literal`
is enabled most of the time.
This module has been soft deprecated for a long time, but since
it was used internally it wasn't throwing deprecation warnings.
Now we can throw a deprecation warning.
Improve the parity between `form_for` and `form_with` by implementing
`form_for` in terms of `form_with`.
Replaces `html_options` transformations with coercion of data into the
shape it needs to be in order to delegate to `form_with`.
In the same spirit, this commit also implements `fields_for` in terms of
`fields`.
Since `<button>` elements translate their `[name]` and `[value]`
attributes to the resulting `<form>` element submission, and are encoded
into the resulting `URLSearchParams` or `FormData` instance, Action View
`FormBuilder` instances should support encoding a method name the same
way it does for other fields.
For instance, consider this HTML:
```html
<button>Publish</button>
<button name="post[draft]" value="true">Save as draft</button>
```
Clicking the "Publish" button would submit the form without encoding any
additional `[name]` and `[value]` pairs.
Clicking the "Save as draft" button would submit the form and encode
`post[draft]=true` into the submission.
This commit changes the `FormBuilder#button` method to interpret a
`Symbol` as the first argument as a method name argument, and encodes
its value based on the form's `model:` or `scope:` value:
```erb
<%= form.button :draft, value: true do %>
Save as draft
<% end %>
end
<%# => <button name="post[draft]" value="true" type="submit"> %>
<%# Save as draft %>
<%# </button> %>
```
Co-authored-by: Rafael Mendonça França <rafael@rubyonrails.org>
The `tag.attributes` helper introduced in [#40657][] is implemented on a
`:nodoc:`-private `TagBuilder` class, so the documentation for the
method is omitted when generating the API Guides HTML pages.
This commit extends the `ActionView::Helpers::TagHelper` module
documentation comment to mention the new attribute building
capabilities, along with some examples.
[#40657]: https://github.com/rails/rails/pull/40657
Some background
---
By default, when a `<form>` is declared without an `[action]` attribute,
browsers will encode a `<form>`'s fields into the _current_ URL.
This can be useful for a `<form method="get">` that operates on the
current page. For example, when filtering search results, a form that
sorts:
```html
<form method="get">
<button name="sort" value="desc">Most to least</button>
<button name="sort" value="asc">Least to most</button>
</form>
```
can operate on a page that is filtered in another way by merging the
`?sort=asc` or `?sort=desc` values _into_ the existing page, which might
have the `?q=...` string set elsewhere.
The problem
---
Prior to this commit, none of the `<form>` construction variations
supported declaring a `<form>` without an `[action]` attribute.
`form_with`, `form_for`, and `form_tag` all default to `url_for({})`
when a `url:` or `action:` option is omitted.
The solution
---
Treat `url: false`, or `action: false` as an escape hatch to signal to
Action View that we don't need to transform the `model:` option or
argument into a Rails route.
Similarly, when calling `button_to` with `false` as the URL options
arguments will construct a `<form>` element without an `[action]`
attribute.
Instead of treating it as an anonymous block, execute the
`ActionView::Base.field_error_proc` within the context of the
`ActionView::Base` instance.
This enables consumer applications to continue to override the proc as
they see fit, but frees them from declaring templating logic within a
`config/initializers/*.rb`, `config/environments/*.rb` or
`config/application.rb` file.
This makes it possible to replace something like:
```ruby
config.action_view.field_error_proc = proc do |html_tag, instance|
<<~HTML.html_safe
#{html_tag}
<span class="errors">#{instance.error_message.to_sentence}</span>
HTML
end
```
With inline calls to Action View helpers like:
```ruby
config.action_view.field_error_proc = proc do |html_tag, instance|
safe_join [ html_tag, tag.span(instance.error_message.to_sentence, class: "errors") ]
end
```
Or with a view partial rendering, like:
```ruby
config.action_view.field_error_proc = proc do |html_tag, instance|
render partial: "application/field_with_errors", locals: { html_tag: html_tag, instance: instance }
end
```
Then, elsewhere in `app/views/application/field_with_errors.html.erb`:
```erb
<%= html_tag %>
<span class="errors"><%= instance.error_message.to_sentence %></span>
```
Infer HTTP verb `[method]` from a model or Array with model as the first
argument to `button_to` when combined with a block:
```ruby
button_to(Workshop.find(1)){ "Update" }
#=> <form method="post" action="/workshops/1" class="button_to">
#=> <input type="hidden" name="_method" value="patch" autocomplete="off" />
#=> <button type="submit">Update</button>
#=> </form>
button_to([ Workshop.find(1), Session.find(1) ]) { "Update" }
#=> <form method="post" action="/workshops/1/sessions/1" class="button_to">
#=> <input type="hidden" name="_method" value="patch" autocomplete="off" />
#=> <button type="submit">Update</button>
#=> </form>
```
Prior to this change, the constructed `<form>` was always submitted with
a `[method="post"]` and _always_ omitted the `<input type="hidden"
name="_method" value="...">` field, regardless of the return value of
the "model" argument's `#persisted?` predicate.
It's `Rails.application.executor.wrap` that is responsible for
clearing request/job local state such as `CurrentAttributes`.
Instead of including an ad hoc helper to clear `CurrentAttributes` it's
better to run the executor so we properly clear other states as well.
However it means all executor hooks now need to be re-entrant.
This module will be a private module in Active Support, this way
if we need to change the behavior of translate in controllers or
views don't forget to change in the other one.
* Make Sprockets more optional, offer Propshaft as alternative
* Whups, local reference
* No longer used
* Spacing
* Need explicit sprockets-rails inclusion now
* Manually require the sprockets railtie
* Don't need these changes right now
* Kick off another build
* Fix tests
* DRY up test
* Require railtie when using sprockets
* Introduce option to skip asset pipeline
* No longer relevant
* Always have to return
* Gone
* Add helper for skip_sprockets?
* Fix guard statement
* Use latest gems
* Include propshaft
* fix tests for #43261 (#43277)
* help fix tests for #43261
skip_sprockets? should not be called on options
:skip_sprockets is no longer a value in the option hash, so
skip_sprockets? should not be called on it
move --asset-pipeline to shared generator
skip_sprockets? is defined on app_base, and used in the plugin
generator to determine whether to add the engine's assets to the dummy
sprockets manifest, so I believe it makes sense to include in both
generators
Because of this change, I also changed the shared test back to testing
against non-sprockets
add skip_sprockets to Gemfile template vars
Mocking skip_sprockets? in app_base generator
fix more generator tests
* use skip_sprockets? everywhere
* Use latest propshaft
* Update `AssetUrlHelper` docs to list both asset pipeline gems (#43328)
* Update to latest Propshaft
* Bump Propshaft again
* Ask for latest
* Use latest propshaft
Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>
Co-authored-by: Richard Macklin <1863540+rmacklin@users.noreply.github.com>
The word "Crazy" has long been associated with mental illness. While
there may be other dictionary definitions, it's difficult for some of us
to separate the word from the stigmatization, gaslighting, and bullying
that often comes along with it.
This commit replaces instances of the word with various alternatives. I
find most of these more focused and descriptive than what we had before.
The ActionView::Helpers::Tags::CheckBox class accepts the `checked` and `include_hidden` options. The documentation for these options is added to the `check_box` methods in the ActionView::Helpers::FormHelper class where they are exposed.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Change `begin_on_monday` to `beginning_of_week`
Change `begin_on_monday` to `beginning_of_week`
Better handle ignored day
Remove inaccurate tests
Improve docs and use DAYS_INTO_WEEK to calc rotation
Remove inaccurate docs
Use Kernel::Float(..., exceptions:false) instead of a rescue block in
ActionView::Helpers::NumberHelper and
ActiveSupport::NumberHelper::NumberConverter to slightly improve
performance.
Also remove documentation that incorrectly states
ActiveSupport::NumberHelper supports the `raise:` option.
Swaps to case/when to highlight the 3 branches better and uses `presence_in`
to cut down on the else branch noise.
`resolve_link_as` added in: eb90b8bc86
Most recent other commit: 46bfd082b0
Made a decision to tweak this as core, don't send cosmetic PRs.
I found an unexpected use of assertion in the block of `assert_raise`
when I implemented https://github.com/rubocop/rubocop-minitest/pull/137.
It is expected to be asserted after an exception is raised in
`assert_raise` block, but in actually it is not asserted after an
exception is raised. Therefore, this PR removes or updates assertions
that have not been asserted after an exception has raised.
This PR will add `rubocop-minitest` and enable
`Minitest/UnreachableAssertion` cop to able similar auto-detection,
but will remove `rubocop-minitest` from this PR if you don't like it.
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
This leaves Ripper tracker as an optional view dependency tracker, but uses the current ERBTracker by default.
Eventually the default can change to the Ripper tracker, but this makes it an optional update for now
Caching something that shouldn't be cached is a potential source of
bugs and security vulnerabilities. For example, one could write a
form helper that outputs a request-specific auth token, only for
the helper to be used inside of a `cache` block.
In the GitHub application, we implemented a caching? method and used
it to raise an error if a specific code path is being cached that
we don't want to be cached.
I've credited its original author, @btoews.
Co-authored-by: Ben Toews <mastahyeti@gmail.com>
Co-authored-by: John Hawthorn <jhawthorn@github.com>
Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
* Update resolve_link_as to include SVG
Preloading SVGs won't work without 'as: image' because of:
`Preload of /packs/media/images/81d035675e31079ea9da.svg was ignored due to unknown “as” or “type” values, or non-matching “media” attribute.`
Preloading SVGs can be useful, for instance when using sprites such as bootstrap-icons.
* Adds test case for asset_tag_helper
* Fix test case -- silly mistake
[xanderificnl + Rafael Mendonça França]
This fixes the `current_page?` helper when the given URL has a trailing
slash, and is an absolute URL or also has query params.
Fixes#33956.
Co-authored-by: Rien Maertens <rien.maertens@posteo.be>
Rafael Mendonça França