@tomhughes requested changes on this pull request.
> + def browse_tag_value_cell(tag)
+ options = {
+ :class => "py-1 border-secondary-subtle border-start",
+ :dir => "auto"
+ }
+
+ lang = tag_language(tag[0])
+ options[:lang] = lang if lang
+
+ content_tag :td, format_value(tag[0], tag[1]), **options
+ end
+
+ def tag_language(key)
+ key[/\Aname:([a-z]{2,3}(?:-[A-Za-z0-9]+)*)\z/, 1]
+ end
+
So which is it? Are you going to put this routine here, or in
`BrowseTagsHelper` and then include that?
What you don't need is to do both...
To be honest at this point you might as well just inline it in
`browse_tag_value_cell` anyway.
> + lang = tag_language(tag[0])
+ options[:lang] = lang if lang
I suspect that assigning `nil` to the option to leave it unset would work, and
that you could change this to:
```suggestion
options[:lang] = tag_language(tag[0])
```
> @@ -72,6 +74,22 @@ def link_follow(object)
"nofollow" if object.tags.empty?
end
+ def browse_tag_value_cell(tag)
+ options = {
+ :class => "py-1 border-secondary-subtle border-start",
+ :dir => "auto"
+ }
+
+ lang = tag_language(tag[0])
+ options[:lang] = lang if lang
+
+ content_tag :td, format_value(tag[0], tag[1]), **options
You might as well just put the options here rather than assigning them to a
variable and then dereferencing it.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6854#pullrequestreview-3946158668
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6854/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
