Skip to content
This repository has been archived by the owner on Jun 16, 2020. It is now read-only.

Replace (some) variables arrays with separate variables #809

Open
sboerrigter opened this issue Sep 5, 2017 · 2 comments
Open

Replace (some) variables arrays with separate variables #809

sboerrigter opened this issue Sep 5, 2017 · 2 comments
Labels

Comments

@sboerrigter
Copy link
Member

I think the syntax to use a variable within an array is too verbose. I prefer to use:

line-height: $line-height-small;

over:

line-height: map-get($line-height, 'small');

I would like to changing (some of the) array variables to seperate variables, to decrease complexity and increase development speed.

@luukdv
Copy link

luukdv commented Sep 6, 2017

Maybe it's cleaner when there are only 1 or 2 variables, but beyond this maps are a lot less verbose and more readable:

$lorem-ipsum-dolor: (
  'sit-amet': 'one',
  'consectetur': 'two,
  'adipiscing-elit': 'three',
  'pellentesque': 'four',
  'ultricies-mi': 'five',
);
$lorem-ipsum-dolor-sit-amet: 'one';
$lorem-ipsum-dolor-consectetur: 'two';
$lorem-ipsum-dolor-adipiscing-elit: 'three';
$lorem-ipsum-dolor-pellentesque: 'four';
$lorem-ipsum-dolor-ultricies-mi: 'five';

I can smell some repetition and the scent is not that pleasant.

@sboerrigter
Copy link
Member Author

sboerrigter commented Sep 6, 2017

You are talking about the defining of the variables, but you do this "repetition" only once. I prefer to make the implementation of variables easier and faster because we do this a lot more during development. That is why I prefer:

$lorem-ipsum-dolor-sit-amet;

over:

map-get($lorem-ipsum-dolor, 'sit-amet')

I don't think we should change this for all variables. But only the variables that we use a lot during development. So I suggests changing things like line-height, but not the breakpoints array.

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

No branches or pull requests

2 participants