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.

Sign up for GitHub

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

Jump to bottom

Method to scan for and register stylesheets #2233

Merged
merged 3 commits into from
Oct 17, 2014
Merged

Method to scan for and register stylesheets #2233

merged 3 commits into from
Oct 17, 2014

Conversation

levithomason
Copy link
Contributor

Problem
As web apps become more dynamic developers are now finding they have the need to load/unload stylesheets dynamically. Currently, less does one initial scan for stylesheets on load. This means less will not parse stylesheets added dynamically after page load, even when calling less.refresh() or less.modifyVars().

Proposed Solution
Make the stylesheet loading code available as a method, returning a promise. On resolve, you can .then() call a less.refresh() or less.modifyVars():

less.registerStylesheets().then(
    function () {
        less.modifyVars(record);
    }
);

@levithomason levithomason mentioned this pull request Oct 16, 2014
Closed
@levithomason levithomason force-pushed the feature/reload-stylesheets branch from 61d730e to 2b8d0b3 Compare October 16, 2014 04:49
This was referenced Oct 16, 2014
Closed
Closed
@levithomason levithomason changed the title Register Stylesheets Promise Method Method to scan for and register stylesheets Oct 16, 2014
@levithomason levithomason force-pushed the feature/reload-stylesheets branch 4 times, most recently from fe474d3 to 7834694 Compare October 16, 2014 05:52
lukeapage
lukeapage reviewed Oct 16, 2014
View reviewed changes
lib/less-browser/index.js
less.sheets.push(links[i]);
}
}
if (i === links.length - 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the same as calling resolve at the end of the function? Would that be simpler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aye aye, fixed. Thanks.

lukeapage
lukeapage reviewed Oct 16, 2014
View reviewed changes
lib/less-browser/index.js
less.refresh(less.env === 'development');
less.registerStylesheets().then(
function () {
less.refresh(less.env === 'development');
Copy link
Member

Choose a reason for hiding this comment

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

Less.refresh does the actual transformation, so does less.refresh need to return a promise and then this line here returns that so the promises are chained?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, my other PR includes the promise for refresh(). Should I merge it and make the changes you propose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heading out, I'll be back on this on Fri.

Copy link
Member

Choose a reason for hiding this comment

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

I'll Look later, I can't figure this out on my mobile..

Copy link
Member

Choose a reason for hiding this comment

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

yeh I got confused between the 2 pull requests.. thought they were 1...

@levithomason levithomason force-pushed the feature/reload-stylesheets branch from 7834694 to d31be57 Compare October 16, 2014 05:59
lukeapage added a commit that referenced this pull request Oct 17, 2014
@lukeapage
Merge pull request #2233 from levithomason/feature/reload-stylesheets
659bf74 
Method to scan for and register stylesheets
@lukeapage lukeapage merged commit 659bf74 into less:2_0_0 Oct 17, 2014
@lukeapage
Copy link
Member

Hope you don't mind, I made a few tweaks.

renamed hasFinished to pageLoadFinished and made it explicit that it will be available immediately (and not changed).
8b42ae3

@levithomason
Copy link
Contributor Author

Awesome thanks! Glad we got it knocked out.

Id like to beef up the browser docs as well. Any where specific you need help?

@lukeapage
Copy link
Member

Fantastic, yes there is a v2 branch on less-docs - there are basic
instructions on the home page then usage has extra info. Anything you can
add would be great. We write in md then it's compiled to html.

@David-Mulder
Copy link

Ehm, so just a practical question, does this allow unloading specific stylesheets as per #771? Because as far as I can see this does not, yet that issue states it has been fixed in this pull.

@levithomason
Copy link
Contributor Author

Hi @Greats. This PR addresses the need to dynamically load and unload stylesheets, albeit using a different method than the one detailed in #771.

This PR allows you to rescan the DOM after having added or removed stylesheets using some traditional templating means. I'm using AngularJS' ng-if directive. Of course, this can be done with any templating languge conditional or a js library like Knockout or jQuery.

After you have the stylesheets you want on the page (not limited to the <head>) you can less.registerStylesheets(). This returns a promise:

less.registerStylesheets().then(
    function () {
        // The new stylesheets have been loaded.
        // Now refresh() or modifyVars() to parse the changes.
        less.refresh();
    }
);

Let me know if you're able to get this working with your use case.

@levithomason levithomason deleted the feature/reload-stylesheets branch October 22, 2014 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@levithomason @lukeapage @David-Mulder