Backport Improve form f.inputs attributes rendering #8446
Conversation
* Improve rendering attributes for f.inputs * Support rails 6.1 but use tag.attributes if available * Use tag.legend instead of "<legend ..."
a9c9c02 to
0db1589
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3-0-stable #8446 +/- ##
=============================================
Coverage ? 99.19%
=============================================
Files ? 194
Lines ? 4963
Branches ? 0
=============================================
Hits ? 4923
Misses ? 40
Partials ? 0 ☔ View full report in Codecov by Sentry. |
0db1589 to
2eee8b3
Compare
There was a problem hiding this comment.
Based on your commit and reviewing the CI error, I didn't understand what happened. Do you know what the cause was? Would it be necessary/helpful to do this helpers change on the default branch as well?
There was a problem hiding this comment.
Correct, I made #8448 to use helpers in both branches. The problem is not evident and only affects Tag resource (edge-case?).
My theory is that v3 expose it because default form is rendered in the same context as a custom one (ResourceController or something similar where tag is an alias of resource attr_reader)
There was a problem hiding this comment.
It wouldn't be the first time we've had a similar problem. I figured that would be the case, where it's some conflict with a resource. I think it makes sense to add that change back in the default branch too. Thank you again for addressing.
|
Note: I'm waiting to merge this because I want to upstream a small fix to default branch first. |
#8439 improved form attributes rendering but introduced an issue if host app has an admin for Tag resource. The problem is that `tag` (`tag.attributes`) refences the resource and not the `TagHelper`. Let's prefix the call with `helpers`.
|
@javierjulio, @amiel I'm going to release a new version with this next week. |
|
Thank you so much! |
Backport #8439 and #8448