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

Add utility function resolveUrl on manager #1993

Merged
merged 1 commit into from Mar 16, 2018

Conversation

@vidartf
Copy link
Member

commented Mar 8, 2018

This is needed to be able to resolve file URLs relative to the widget's notebook.

Example: My widget needs to load an image at the path 'img/earth.jpg'. This is implied to be relative to the path of the current notebook (I cannot see a case where this wouldn't be what the user would expect, but I'm sure someone will come along at some point). In classic notebook, this works out of the box, since the url always incorporates the location of the notebook. For jupyter lab, this is not the case. By adding a utility function on the widget manger, we add a frontend-agnostic way of resolving the URL.

Add utility function resolveUrl on manager
This is needed to be able to resolve file URLs relative to the widget's
notebook.
@SylvainCorlay

This comment has been minimized.

Copy link
Member

commented Mar 8, 2018

Thanks @vidartf. This should solve jupyter-widgets/ipyleaflet#120, which is exactly what you described.

@maartenbreddels

This comment has been minimized.

Copy link
Member

commented Mar 14, 2018

@astrofrog I think you also needed this a while back, and I gave you a dirty hack for it, this looks much cleaner

@vidartf vidartf requested a review from jasongrout Mar 14, 2018

@jasongrout

This comment has been minimized.

Copy link
Member

commented Mar 16, 2018

Thanks!

@jasongrout jasongrout merged commit 45f2c3b into jupyter-widgets:master Mar 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jasongrout jasongrout added this to the Minor release milestone Mar 16, 2018

@vidartf vidartf deleted the vidartf:resolveurl branch Mar 16, 2018

@jasongrout jasongrout modified the milestones: Minor release, 7.2 Mar 19, 2018

@vidartf

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2018

Should this have been using getDownloadUrl instead of resolveUrl on UrlResolver?

@vidartf

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2018

Or rather, in addition to.

@vidartf

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2018

Mainly to be future proof in case anybody adds another Drive with an overridden getDownloadUrl.

@jasongrout

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

Good question. Can you open another issue so we can discuss, and possibly target to a 7.2.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.