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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add standard img tag attribute support. #353

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

Conversation

Cleanshooter
Copy link
Contributor

While using this tool I came across the need to use the srcset attribute on my img tag but found that the attribute was being applied to the figure instead of the img tag. This change should add support for all standard img tag attibutes so they go where they belong.

Example:

<figure class="image jumbotron-img is-16by9" srcset="/images/homepage-main-768.jpg 768w, /images/homepage-main-1024.jpg 1024w, /images/homepage-main-1216.jpg 1216w, /images/homepage-main-1408.jpg 1408w, /images/homepage-main.jpg 6720w" sizes="100vw">
  <img class="" src="/images/homepage-main.jpg" alt="Homepage Main Image">
</figure>

Should output as:

<figure class="image jumbotron-img is-16by9" >
  <img class="" src="/images/homepage-main.jpg" alt="Homepage Main Image" srcset="/images/homepage-main-768.jpg 768w, /images/homepage-main-1024.jpg 1024w, /images/homepage-main-1216.jpg 1216w, /images/homepage-main-1408.jpg 1408w, /images/homepage-main.jpg 6720w" sizes="100vw">
</figure>

This is my first attempt at modifying code on this project so please feel free to provide corrections or request adjustments. I'm not really familiar with TypeScript so I didn't modify that part and was unsure if I needed to.

Ohhh and the prettier mod was to remove some line ending issue I was seeing (similar to: https://stackoverflow.com/questions/53516594/why-do-i-keep-getting-delete-cr-prettier-prettier). I use Windows for dev, I'm guessing you use Mac. Let me know if it causes issues on your end. 馃槃

@kennethnym kennethnym self-requested a review August 13, 2021 13:52
Copy link
Collaborator

@kennethnym kennethnym left a comment

Choose a reason for hiding this comment

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

I think having a prop that stores all props for <img> will be a better solution that manually copying them:

<Image imgProps={{}} />

I still don't think it's the most elegant approach though as it can be confusing.

Copy link
Collaborator

@kennethnym kennethnym left a comment

Choose a reason for hiding this comment

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

I changed my mind. Your approach is fine, but can you also update the TypeScript typedef for Image as well? Thank you.

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

Successfully merging this pull request may close these issues.

None yet

2 participants