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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] feat: support remote images #642

Closed
wants to merge 73 commits into from

Conversation

lessp
Copy link
Member

@lessp lessp commented Nov 13, 2019

While experimenting with Revery I'm using a separate fork where I've, kind of naively, added support for remote-images. I'm opening so we can discuss and take the implementation further.

Questions:

  • What to do about errors?

Would close #537

@bryphe
Copy link
Member

bryphe commented Nov 14, 2019

Wow, this would be so nice to have 😍 Thanks @lessp for looking at this

@lessp
Copy link
Member Author

lessp commented Nov 23, 2019

I added Revery_Fetch which currently just wraps fetch-native-lwt in https://github.com/lessp/reason-fetch/.
I don't know if this is something we want to add now, it might be something for a later discussion 🤷‍♂

For Windows-support the main-blocker is:

ocsigen/lwt#745

which will solve:

lessp/fetch#18
lessp/fetch#14

And we'd also obviously want to handle the pin to anmonteiro's in order for it not to leak the resolutions.

https://github.com/revery-ui/revery/pull/642/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R56-R58


Also, I'm not sure how to make this work in JS, but feels like it shouldn't be too hard?

@lessp lessp changed the title feat: support remote images [WIP] feat: support remote images Nov 23, 2019
@lessp lessp marked this pull request as ready for review November 23, 2019 10:45
@lessp
Copy link
Member Author

lessp commented Mar 22, 2020

Closing in favour of #796

@lessp lessp closed this Mar 22, 2020
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.

Load image from absolute path/url
4 participants