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

Allow disabling error markup generation on a per-field basis #46666

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

camertron
Copy link
Contributor

This PR is a replacement for #46618

Motivation / Background

I'm working on a forms framework on the Primer team at GitHub that allows rendering forms declaratively. My pair and I noticed today that Rails will wrap invalid inputs and labels with a <div> containing a class of field_with_errors, ostensibly allowing for custom styling of invalid fields. We also learned this behavior can be customized by setting the application-level ActionView::Base.field_error_proc option.

Our framework automatically handles styling invalid fields in accordance with our design system. The additional wrapper <div>, while something we can work around, unexpectedly led to a visual regression today in our testing:

image

As you can see, the little arrows on the right-hand side of the select list are outside the red outline. The arrows are implemented as an :after selector and live here. The additional <div> causes the :after selector to place the mask after the <div> instead of after the <select>.

Detail

My pair and I were unable to find any field-level option to disable wrapping, so I have submitted this PR. You can now pass generate_error_markup: false to form helpers to skip wrapping fields with the aforementioned <div>:

<%= form_with(model: user) do |f| %>
  <%= f.text_field(:name, generate_error_markup: false) %>
<% end %>

Open Questions

  • As the generate_error_markup: option can be passed to all form helpers and form builder methods, I'm not sure where the most appropriate place is to document the changes. Is someone able to help me with that?

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.
  • CI is passing.

@rails-bot rails-bot bot added the actionview label Dec 7, 2022
@camertron camertron changed the title Disable active model error markup Allow disabling error markup generation on a per-field basis Dec 7, 2022
@camertron camertron force-pushed the disable_active_model_error_markup branch from 099c79b to 7b379bc Compare December 9, 2022 19:01
@camertron camertron force-pushed the disable_active_model_error_markup branch 2 times, most recently from 757cf90 to 20aed2a Compare September 29, 2023 22:38
Rails wraps form inputs in a <div class="field_with_errors"> if
the associated active record field is invalid. This is not always
desirable, and can only be customized application-wide by setting
ActionView::Base.field_error_proc.

Now developers can opt out of the default behavior on a per-field
basis by passing generate_error_markup: false to form helpers, eg:

<%= form_with(model: user) do |f|
  <%= f.text_field(:name, generate_error_markup: false) %>
<% end %>
@camertron camertron force-pushed the disable_active_model_error_markup branch from 20aed2a to f452bc9 Compare October 16, 2023 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant