search

diegomura / react-pdf

📄 Create PDF files using React
https://react-pdf.org
MIT License
14.81k stars 1.17k forks source link

Responsive images (handle multiple image sizes & resolutions) #481

Open DuncanMacWeb opened 5 years ago

DuncanMacWeb commented 5 years ago

Here are some ideas to improve how we handle images, that I had while thinking about #478:

Handling multiple image resolutions

The web has a standard that we could draw on to handle multiple image resolutions: the srcset and sizes attributes. Might this be useful to, for example, output a smaller PDF for screen than for print?

~Upgrade from isomorphic-fetch and alias fetch~

(separated out to #486)

~Also it might be good to take the opportunity to upgrade to cross-fetch which seems to be better maintained than isomorphic-fetch. We could alias fetch to cross-fetch at the same time:~

diegomura commented 5 years ago

Thanks for the ideas @DuncanMacWeb ! Both sound good to me. The second one should be really straight forward. The first one can be a bit more tricky to implement, but we can maybe use the same media-engine lib that we already use fo media queries. Would be happy to make these changes but if you want feel free to do it!

DuncanMacWeb commented 5 years ago

No worries! I can definitely upgrade the library but don’t have much bandwidth to tackle the full responsive images spec right now... will give it some further thought though :)

diegomura commented 5 years ago

I just did the cross-fetch changes in a separate PR. Hope you don't mind.

That change is great, specially because cross-fetch solves some issues un React Native that isomorphic-fetch, for future RN integration.

I kind of needed this change before moving forward with #485, so I can now have better (and fastest) tests using jest-fetch-mock

diegomura commented 5 years ago

My only concern here is that isomorphic-fetch is a fbjs dependency, which is also a react-reconciler dependency. So there's no really a way of getting rid of isomorphic-fetch completely right? what do you think @DuncanMacWeb ?

DuncanMacWeb commented 5 years ago

@diegomura thanks! I’ve replied over at #486... so let’s change this issue to cover the multiple image resolutions / responsive images only.