JarekToro / responsive-layout

The layout to beat them all!
https://vaadin.com/directory#!addon/responsive-layout
57 stars 24 forks source link

Make size border configurable #36

Open dve opened 7 years ago

dve commented 7 years ago

It would be nice to have the values configurable which decide if you are in XS, SM, MD or LG mode.

For this I would suggest to extract the values to variables like $xs-max-width and don't include the compiles css into the addon, but add a Vaadin-Stylesheets entry to the MANIFEST.MF

By the way: is there a reason that sometimes px is used as a unit and sometimes em?

JarekToro commented 7 years ago

That's actually what I originally wanted it already written in scss for this reason, but couldn't find much documentation on how to implement it. What is the syntax of the manifest file?

For the sometimes 'px' sometimes 'em': I like to keep web browsers on there toes so they never know what might happen next.

dve commented 7 years ago

The "documentation" is in the wiki, down in the comments: https://vaadin.com/wiki/-/wiki/Main/Packaging+SCSS+or+CSS+in+an+add-on

For the vaadin team developers seems to be what browsers are to you ;-)

dve commented 7 years ago

Hi jarek,

i've found some time to work on this, but I ran into the problem that the vaadin sass compile seems to not work with & in the scss files.

Do you have any knowledge about that - my scss knowlage is quite small...

JarekToro commented 7 years ago

How are you using the &?

JarekToro commented 7 years ago

Thank you for the hard work btw.

dve commented 7 years ago

How are you using the &?

I try to use your scss as a "addon-theme" which gets compiled by the vaadin sass compiler

JarekToro commented 7 years ago

Yes I remember that vaadin a compiler had an issue with this. There is a ticket about it somewhere let me try and find it

JarekToro commented 7 years ago

https://vaadin.com/forum#!/thread/11403911

dve commented 7 years ago

No good news - the "solution" would be to "refactor" the styles to remove the &?

JarekToro commented 7 years ago

Awesome! :| well ill have to find someway of replacing them

dve commented 7 years ago

Another idea:

  1. Extract the the border definitions to sass variables
  2. Set the variables with magic numbers
  3. Compile the code to css like you do now
  4. Replace the magic numbers with sass variables, rename the file to scss

The advantage would be to not messing up the code with removing the & but we have to pay with a more complicated build process

JarekToro commented 7 years ago

I like the way you think!

JarekToro commented 7 years ago

Ok I just pushed the changes. There is a workaround.scss that has 3 variables

$sm-breakpoint: 48em;
$md-breakpoint: 62em; 
$lg-breakpoint: 75em;

as of now they need to be em because of some math that the sass file is doing

dve commented 7 years ago

Hi Jarek,

it starts to get frustrating - vaadin sass sems to not support variables in media queries... It seems to be a sass 3.2 feature and it looks like the vaadin compiler is not really up to date https://github.com/vaadin/sass-compiler/issues/308

JarekToro commented 7 years ago

That is ridiculous, hopefully soon they will allow this, Thanks for the effort put in.