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

Make fonts always relative to project.less is not good idea #13478

Closed
normanzb opened this issue May 1, 2014 · 8 comments · Fixed by #13542
Closed

Make fonts always relative to project.less is not good idea #13478

normanzb opened this issue May 1, 2014 · 8 comments · Fixed by #13542
Labels
Milestone

Comments

@normanzb
Copy link

normanzb commented May 1, 2014

veto to #10941 , the guy report the issue must because he is compiling with --relative-urls turned on, that's why fonts folder are relative to bootstrap.less

whether or not relative should be controlled by lessc parameter --relative-urls not by hard coding it.

it would be actually good idea to make sure fonts are relative to bootstrap.less (when --relative-urls is used) as i don't have to copy over the fonts from bower folder to project folder, a simple bower install should do the job.

@jhnns also mentioned that the change cause rootpath options useless in the thread of #10941 .

@cvrebert cvrebert added the css label May 1, 2014
@cvrebert
Copy link
Collaborator

cvrebert commented May 1, 2014

Technically, the icon font path is currently relative to the location of the generated CSS file.

@normanzb
Copy link
Author

normanzb commented May 1, 2014

@cvrebert yes, and that is why i veto to the change, because it makes use of bootstrap really inconvenient, everytime a new .css is generated I have to copy over the fonts to sit aside.

@cvrebert
Copy link
Collaborator

cvrebert commented May 1, 2014

All you need to do is adjust the @icon-font-path variable, which will be documented better in v3.2.0 (see #13353, #13435).
Not saying that there isn't room for improvement though.

@normanzb
Copy link
Author

normanzb commented May 1, 2014

still that require changing the --modify-var, and it is not how less suggested to do and not intuitive.

  • when using less people expect changing the rootpath will fix every path but in the case of bootstrap, we will have to do extra setting.

  • says what if I dynamically load in all my less files rather than pre-compile them? (using less in browser ) and all my .htm files are in different folder?

    bower_component/bootstrap
    foo/a.html
    bar/foo/b.htm

    if bootstrap undo the change, all I have to do in this case is to change the path which include bootstrap ONCE each page, but now i have extra steps to make sure the modifyVars matches my bootstrap path as well.

  • I am not a fan of changing the variable, because i think the variable is part of bootstrap's internal, as a well designed system, it should encapsulate and do not encourage people to modifying its internal rather than expose every piece of its internal and letting user tweak it.

maybe bootstrap has its own reason to stay as it is now, if you insist, i will close the issue, thx.

@cvrebert
Copy link
Collaborator

cvrebert commented May 1, 2014

One complicating factor is that some Bower tools reorganize Bootstrap's directory structure, which can break relative path references.

normanzb added a commit to englishtown/gudstrap that referenced this issue May 2, 2014
…e same variable, we should do the same thing for the time being.

However let's keep an eye on issue twbs#13478, see if they change this.
@sokra
Copy link

sokra commented May 8, 2014

👍 for reverting the "fix". Revert: sokra@88b3032

You'll need relative urls when @importing bootrap from other less files.

@cvrebert
Copy link
Collaborator

cvrebert commented May 8, 2014

X-Ref: yatskevich/grunt-bower-task#21

@mdo mdo added this to the v3.2.0 milestone May 8, 2014
@mdo mdo closed this as completed in 510f4fe Jun 8, 2014
mdo added a commit that referenced this issue Jun 8, 2014
@mdo mdo mentioned this issue Jun 8, 2014
@jhnns
Copy link

jhnns commented Jun 9, 2014

Nice! 👍

@twbs twbs locked and limited conversation to collaborators Jun 9, 2014
stempler pushed a commit to stempler/bootstrap that referenced this issue Nov 4, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants