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

Fix assets by using relative URLs to point ot them. #1726

Merged
merged 2 commits into from
Jul 17, 2020

Conversation

tvst
Copy link
Contributor

@tvst tvst commented Jul 17, 2020

fixes #1725

@tvst tvst requested review from nthmost and kantuni July 17, 2020 00:23
@tvst tvst requested a review from a team as a code owner July 17, 2020 00:23
@tvst tvst force-pushed the fix-assets branch 3 times, most recently from 98d9e4b to 726bc53 Compare July 17, 2020 05:32
Copy link
Contributor

@nthmost nthmost left a comment

Choose a reason for hiding this comment

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

Confirmed that this commit effectively reverts a problematic change.

@kantuni
Copy link
Collaborator

kantuni commented Jul 17, 2020

@tvst This will break /assets endpoint that custom components use:

def get_assets_dir():

@tvst
Copy link
Contributor Author

tvst commented Jul 17, 2020

@kantuni is there a test for this? If not, how do I test?
(Also, we should add a test here)

@tvst tvst mentioned this pull request Jul 17, 2020
@tvst
Copy link
Contributor Author

tvst commented Jul 17, 2020

Henrikh responded:

I don’t think we have tests for the assets endpoint

It’s not used currently, so feel free to merge it (as it’s a release blocker)

@tconkling
Copy link
Contributor

This will be used by the Streamlit.loadCustomCSS() function we'll be adding to the component template. But we punted on this for the launch, because there were some layout issues. There aren't any tests for it yet (but - @kantuni, could we easily write one that just tries to access /assets/streamlit.css?)

@tvst tvst merged commit a20aba7 into streamlit:develop Jul 17, 2020
@tvst tvst deleted the fix-assets branch July 17, 2020 23:15
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.

Regression: missing icons/fonts when deployed under a subpath (not "/")
4 participants