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 size border configurable #36

Open
dve opened this issue Mar 17, 2017 · 15 comments
Open

Make size border configurable #36

dve opened this issue Mar 17, 2017 · 15 comments
Labels

Comments

@dve
Copy link

dve commented Mar 17, 2017

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
Copy link
Owner

JarekToro commented Mar 17, 2017

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
Copy link
Author

dve commented Mar 17, 2017

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
Copy link
Author

dve commented May 22, 2017

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
Copy link
Owner

How are you using the &?

@JarekToro
Copy link
Owner

Thank you for the hard work btw.

@dve
Copy link
Author

dve commented May 23, 2017

How are you using the &?

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

@JarekToro
Copy link
Owner

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
Copy link
Owner

@dve
Copy link
Author

dve commented May 26, 2017

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

@JarekToro
Copy link
Owner

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

@dve
Copy link
Author

dve commented May 27, 2017

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
Copy link
Owner

I like the way you think!

JarekToro added a commit that referenced this issue May 30, 2017
@JarekToro
Copy link
Owner

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
Copy link
Author

dve commented Jun 13, 2017

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 vaadin/sass-compiler#308

@JarekToro
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants