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

Support bootstrap glyiphicons without hardcoding public path #207

Merged
merged 2 commits into from
Jul 21, 2016

Conversation

kwelch
Copy link
Collaborator

@kwelch kwelch commented Jul 21, 2016

So this removes the public path explicitly set causing issues (#181 & #205)

This is a very hacky solution in my mind.

Special Notes

  • Adding bootstrap to dependancies => this was intention since I didn't expect the bootstrap part to "live"
  • I was able to run on external device, but I was not seeing bootstrap icons re-tested and this works

@coveralls
Copy link

coveralls commented Jul 21, 2016

Coverage Status

Coverage remained the same at 98.501% when pulling 4029e8b on kwelch:bootstrap-support into 8c5a173 on coryhouse:master.

@coryhouse coryhouse changed the title fixes bootstrap glyiphicons without hardcoding public path Fix bootstrap glyiphicons without hardcoding public path Jul 21, 2016
@coryhouse coryhouse changed the title Fix bootstrap glyiphicons without hardcoding public path Support bootstrap glyiphicons without hardcoding public path Jul 21, 2016
/**
* Created by welchk on 7/20/16.
*/
__webpack_public_path__ = window.location.protocol + "//" + window.location.host + "/";
Copy link
Owner

Choose a reason for hiding this comment

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

Where is this variable used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is dynamically sets the public path at runtime allowing proper resolving for loaders.

Found it in the issue for relative paths in style-loader.
Further explaination: webpack-contrib/style-loader#96 (comment)

Honestly, I am not a fan of this outside of the fact that it does work. This will allow proper support icon support to fix #125, but it also does so in a way that does not cause other breaking changes as seen in #181 & #205.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, I see that webpack_public_path is a magic global.

I'd suggest renaming bootstrap-public-path to webpack-public-path and adding a comment inside that explains why its being set dynamically and includes a link to relevant issues and the docs that explain what webpack_public_path is.

Thanks so much for all your hard work on this Kyle! 💯

@kwelch
Copy link
Collaborator Author

kwelch commented Jul 21, 2016

I can revert the bootstrap specific changes if you want to move forward with this.

@coveralls
Copy link

coveralls commented Jul 21, 2016

Coverage Status

Coverage remained the same at 98.501% when pulling 2f6f679 on kwelch:bootstrap-support into 8c5a173 on coryhouse:master.

@coryhouse coryhouse merged commit 092151e into coryhouse:master Jul 21, 2016
@coryhouse
Copy link
Owner

Great work Kyle! Thanks for all your help on this! 👍

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.

3 participants