-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
Draft the beefed up responsive meta tag #835
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one suggestion! Thank you for doing this
src/lucky/tags/specialty_tags.cr
Outdated
@@ -74,4 +75,14 @@ module Lucky::SpecialtyTags | |||
how_many.times { raw(" ") } | |||
view | |||
end | |||
|
|||
private def build_content_attr_value(options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you name this something like build_responsive_meta_tag_content_attr
. So that it is clear it is for the responsive_meta_tag? We may need something like this in the future for other methods, but until then I think being specific will add clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would build_viewport_properties
be ok? If so, then I think it's ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Closes #828. The docs will have to be updated, I wanted to get some feedback first.
Checklist
crystal tool format spec src
./script/setup
./script/test