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

Incorrect label for belongs_to selects #1929

Closed
4 of 10 tasks
xeron opened this issue Aug 31, 2023 · 5 comments · Fixed by #2758 or #2762
Closed
4 of 10 tasks

Incorrect label for belongs_to selects #1929

xeron opened this issue Aug 31, 2023 · 5 comments · Fixed by #2758 or #2762
Labels
Bug Something isn't working Edge case It's not really a bug but an edge-case that might pop up from time to time. Good first issue Good for newcomers

Comments

@xeron
Copy link

xeron commented Aug 31, 2023

Describe the bug

When editing or creating new records with belongs_to fields label's for value doesn't match the input's id value.

In Chome inspector Issues tab the following messages appear:

The label's for attribute doesn't match any element id. This might prevent the browser from correctly autofilling the form and accessibility tools from working correctly.
To fix this issue, make sure the label's for attribute references the correct id of a form field.

Screenshot 2023-08-31 at 4 44 11 AM

Steps to Reproduce

  1. Create 2 models, with has_many / belongs_to relationship
  2. Create avo resource with belongs_to field
  3. Go to new or edit view of the object which uses belongs_to
  4. Try clicking a label for belongs_to field
  5. Correct select input won't be focused

Expected behavior & Actual behavior

This config generates the following HTML:

<label for="result_event">Event</label><select  name="result[event_id]" id="result_event_id"><option value="">Choose an option</option>
<option value="">Event 1</option>

You can see label's for is result_event but input's id is result_event_id.

Expected to see the same id in label's for and in input's id.

Models and resource files

class Event < ApplicationRecord
  has_many :results, dependent: :destroy
end
class Result < ApplicationRecord
  belongs_to :event, touch: true
end
class ResultResource < Avo::BaseResource

  field :event, as: :belongs_to

end

System configuration

Avo version: 2.40.0
Rails version: 7.0.7.2
Ruby version: 3.2.2

License type:

  • Community
  • Pro

Are you using Avo monkey patches, overriding views or view components?

  • Yes. If so, please post code samples.
  • No

Screenshots or screen recordings

N/A

Additional context

N/A

Impact

  • High impact (It makes my app un-usable.)
  • Medium impact (I'm annoyed, but I'll live.)
  • Low impact (It's really a tiny thing that I could live with.)

Urgency

  • High urgency (I can't continue development without it.)
  • Medium urgency (I found a workaround, but I'd love to have it fixed.)
  • Low urgency (It can wait. I just wanted you to know about it.)
@Paul-Bob
Copy link
Contributor

Hi @xeron good catch! My browser do not complains about it, I'm using Firefox. Also tried with Chrome, didn't complained at all.

Correct select input won't be focused

What do you mean? It works as expected on my end.

Anyway good catch again, the label's for should have the same value as the input's id

@Paul-Bob Paul-Bob added the Edge case It's not really a bug but an edge-case that might pop up from time to time. label Aug 31, 2023
@xeron
Copy link
Author

xeron commented Aug 31, 2023

Also tried with Chrome, didn't complained at all.

Sorry, not in the console, issues tab:

Screenshot 2023-08-31 at 4 44 11 AM

I'll make it more clear in the bug report.

What do you mean? It works as expected on my end.

For belongs_to fields select dropdown is not focused when you click on related label because of the id mismatch.

@Paul-Bob Paul-Bob added Bug Something isn't working Good first issue Good for newcomers labels Aug 31, 2023
@Paul-Bob
Copy link
Contributor

Thanks you for the clarification!

@xeron
Copy link
Author

xeron commented Apr 29, 2024

It's been a while since I reported this. Could someone take a look? I checked the code but I don't really understand where this part comes from.

@xeron
Copy link
Author

xeron commented Apr 29, 2024

Well I know select id comes from here, but if I change it to @field.id it fixes the label matching but breaks the form:

<%= @form.select @field.id_input_foreign_key, @field.options,

Something like this fixes the issue but I feel like this is not a proper fix:

diff --git a/app/components/avo/field_wrapper_component.html.erb b/app/components/avo/field_wrapper_component.html.erb
index e044301b..61b22803 100644
--- a/app/components/avo/field_wrapper_component.html.erb
+++ b/app/components/avo/field_wrapper_component.html.erb
@@ -11,7 +11,8 @@
     "md:w-64": !compact?,
   }), data: {slot: "label"} do %>
     <% if form.present? %>
-      <%= form.label field.id, label %>
+      <% id = field.class == Avo::Fields::BelongsToField ? "#{field.id}_id" : field.id %>
+      <%= form.label id, label %>
     <% else %>
       <%= field.name %>
     <% end %>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Edge case It's not really a bug but an edge-case that might pop up from time to time. Good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants