html-next / flexi

Just a layout framework. Design for cross-platform with ease.
https://flexi.readme.io/docs
MIT License
219 stars 36 forks source link

Parsing of breakpoints #37

Closed yankeeinlondon closed 7 years ago

yankeeinlondon commented 8 years ago

Just trying out flexi and it was working but then I added ember-cli-sass and it appears that has broken it. For instance:

SASS:

<box xs="3 vertical">

Get's converted to:

<box class="col-xs-3,vertical vertical-xs">

Furthermore, if I add an explicit class definition things get worse:

SASS:

<box xs="3 vertical" class='heading-bar'>

Is converted to:

<box>
yankeeinlondon commented 8 years ago

Strange, i've tried to reverse this by removing both ember-cli-autoprefixer and ember-cli-sass but it's still working as stated above (it did work initially).

yankeeinlondon commented 8 years ago

yeah it seems I mis-identified the cause of the problem but it is a problem. It translates xs=3 appropriately to .col-xs-3 but multiple items are not parsed correctly. Should be pretty easy to reproduce.

yankeeinlondon commented 8 years ago

Really like the library and having used it for a few more days I looked back on this issue and realized that it was making a rookie syntax error. I'm closing this as a result.

runspired commented 8 years ago

@ksnyde if it's something I can throw a better error for, I'd like to. Could you show me the syntax you were mistakenly using?

yankeeinlondon commented 8 years ago

Ok, I had a dream about this comment. Odd but I won't complain about sleep productivity hacks. Re-opening and changing title to make it more suitable for where this thread has gone. Will summarize in the next post.

yankeeinlondon commented 8 years ago

Ok, so initially when I closed this I did this because I had been a mis-lead a bit by the documentation. Specifically I saw this:

image

So in my initial spike I literally typed this in and this does run into the parsing issues which I mentioned at the top of this issue. So arguably it is a real bug but ...

In practice it could be argued you would only ever put one value per breakpoint. For instance, the following make good sense and work:

If there is ever only one value for the breakpoint properties then there isn't strictly a parsing problem but the documentation should use an example which wouldn't work. Also, I do wonder, can you actually classify justification and wrap by breakpoint value. If yes, that's exciting, but then there is indeed a parsing problem.

runspired commented 8 years ago

This definitely sounds like a bug, but should be caught by test coverage (and I've been using this syntax). The documentation is correct that this is possible. What version of Ember are you on?

runspired commented 8 years ago

@ksnyde wait, are you using curly syntax? {{box ? It should be angle bracket.

runspired commented 8 years ago

Added a new integration test, confirmed a bug, added a debug mode for better debugging the attribute conversion, and got most a fix in place, will send out a patch later today.

runspired commented 8 years ago

Fixed by 9d39e88 and releases as version 1.1.4

yankeeinlondon commented 8 years ago

Great. Will give this a try today. Thanks. Oh, btw, I am using angle brackets, my example was wrong, habits die slow. :)

yankeeinlondon commented 8 years ago

I'm going to reopen this because as I found out today, ordering matters in an unexpected way. Specifically:

<hbox xs="hidden" lg="2 visible">

The above works fine.

<hbox xs="hidden" lg="visible 2"> 

This crashes broccoli with the following error:

Flexi#attribute-conversion:: 'NaN' is not a valid value for lg.

I am using flexi version 1.1.6 (the latest currently)

yankeeinlondon commented 8 years ago

Hmmm. Guess I can't reopen this.

runspired commented 8 years ago

I think this is fixed on latest but it's also got a different bug. Was supposed to be 1.2 prerelease but ended up getting tagged as a patch. Will push new patch this morning.

runspired commented 8 years ago

This may be fixed with 1.1.7 but I have not been able to put a test together for it yet.

kybishop commented 7 years ago

I've used the previously borked order in the wild without issue. Closing this out in favor of https://github.com/html-next/flexi-dsl/issues/5 (create tests for attribute value order)