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 float classes #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add float classes #10

wants to merge 1 commit into from

Conversation

Shebo
Copy link

@Shebo Shebo commented Jan 19, 2016

This update is creating 10 more classes for each existing class: x.1-x.9. giving user more precise classes.

This update is creating 10 more classes for each existing class: x.1-x.9. giving user more precise classes.
@joaocunha
Copy link
Owner

Hi @Shebo. Can you give an use case where this would be needed?

@Shebo
Copy link
Author

Shebo commented Jan 19, 2016

Yeah sure. The difference between vwfs1 class and vwfs2 is way too much, 1vw is a lot (almost 20px on 1080 screens). If someone is in need for 1.5vw, he can now use it.

Actually, this change is based on a real case. Earlier today a friend of mine wanted to use your lib but he needed some font sizes between 1 and 2. I made this change originally for him, but then realized he's not the only one who can benefit from it, hence the pull request.

@joaocunha
Copy link
Owner

@Shebo I see what you mean. It's indeed not the first PR that tries to address this use case - which is a valid one.

The problem is that due the way the library works (it's event-less), nested for loops could be really expensive performance-wise.

I'm happy to accept a PR that addresses this issue, but I'm quite worried about the performance implications on a real website. Maybe we can benchmark the before and after?

@Shebo
Copy link
Author

Shebo commented Jan 19, 2016

I agree that this is not a perfect solution, it's quick n' dirty. Of course that a second nested loop will be expensive. Here's a quick test:
http://jsperf.com/third-nested-loop/5

Looping 100xn times on every load/resize is a heavy operation as is, 10x100xn not helping in any form.
I think there are no perfect solution without changing whole lib structre.

One possible course of action is to to have 1 nested loops that will run 10x100 times, like this:
for (var range = 0; range <= 100; range=sumFloat(range, 0.1))
It's not saving anything except another nested loop (test is showing no improvment: http://jsperf.com/third-nested-loop/6).

Another way is to make the loop asynchronous (like in jquery's/loadsh's/underscore's foreach), thus the browser loading time will not depend on these loops.

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.

2 participants