Skip to content
This repository was archived by the owner on Jun 11, 2022. It is now read-only.

Add alt and class #4

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add alt and class #4

wants to merge 6 commits into from

Conversation

levibrown
Copy link
Member

adds alt attr to the picture tag element and the ability to add a class.

%s(tiny@2x) => "640x",
%s(small@2x) => "960x",
%s(medium@2x) => "1536x",
%s(large@2x) => "2000x",
%s(large@2x) => "3200x"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are there two large@2x's? I think that may want to be changed to huge?

To add a class to the picture tag:

```erb
<%= picture_tag(image_path, picture_class: "some_class"}) %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@levibrown I think it would be better if this behaved like image_tag. Namely, the html class and alt should be passed in options with keys class and alt. http://api.rubyonrails.org/classes/ActionView/Helpers/AssetTagHelper.html#method-i-image_tag

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jessicard
Copy link

I added a commit to fix the readme large/huge typo. @jlsuttles, thoughts on the rest of the PR?

@jlsuttles
Copy link
Contributor

@levibrown Hey! Are you using this branch? We should probably get it merged in.

@levibrown
Copy link
Member Author

@jlsuttles As far as I know nobody is using this branch. Should be good to merge.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants