search

nitish24p / react-worker-image

React component to fetch image resources via web workers 🤖🤖
MIT License
232 stars 18 forks source link

[Opinionated] Add alt attribute & remove container #13

Open kurtextrem opened 6 years ago

kurtextrem commented 6 years ago

I've added an alt attribute, because "Placeholder" and "Worker" as alt is not really nice. Second thing I've done is remove the container, because if you're using <ImageWorker> as replacement for <img> it should not become a <div>.

nitish24p commented 6 years ago

So i converted it to a div because if someone supplies a placeholder as a div and then the image loads, so we will toggle from a block element to an inline element, and that may break UI. Hence i thought of wrapping it in a block element div regardless. If one wants to make the parent an inline then you can add a containerClass prop.@manjula91 

display: inline-block
manjula-dube commented 6 years ago

Looks Good

@nitish24p I think we can merge it :)

nitish24p commented 6 years ago

One comment, and eslint..

nitish24p commented 6 years ago

can you run npm run eslint to see the lint errors and fix those

kurtextrem commented 6 years ago

Fixed!

But I think it should be added to the readme, because as I said I'd expect an <img> replacement to be inline too.