-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
src/_settings.scss
Outdated
'cookie-banner': 50, | ||
'ad-smartbanner': 10, | ||
'card-article-icon': 1, | ||
'form-autocompleted-suggests': 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This two may or may not collide? I understand you are just simply refactoring out... but its worth setting form-autocompleted-suggest at least to 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would do it, but as you say, it wasn't just a refactor, and it should be tested carefully. Firstly I wanted to set the basis in order to have all z-index
values sorted, it's much much more than what we have now 😄 .
@@ -0,0 +1,3 @@ | |||
@function z($layer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, why not z-index
?
I really dislike the approach of making everything short... we are not saving bytes in the compilation and makes stuff harder to understand :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not about saving bytes, it's about not writing names too long, the same reason we're following emmet abbreviations for variable names: https://docs.emmet.io/cheat-sheet/. I have just followed this standard but I'm open to change it if the most of @SUI-Components/developers don't feel comfortable with it in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess is a discussion for a total different than this PR so go ahead.
I just that I personally don't understand this decision at all 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emmet is a global convention well know for naming of common aspects.
IMO we should not create new shorthands for specific stuff.
We should stick to the ones of the convention as exceptions.
Not really related to this task, of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the emmet approach. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally would prefer a descriptive name, at least for functions
@@ -156,6 +158,15 @@ $bdrs-small: ceil($bdrs-base / 2) !default; | |||
$bxsh-spread: 3px !default; | |||
$bxsh-base: 0 1px $bxsh-spread rgba($c-black, .15) !default; | |||
|
|||
// Layers (z-index) sorting. | |||
$z-layers: ( | |||
'modal-basic': 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to have these sorted alphabetically as it seems that's not scalable at all having those random. :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it really makes more sense to have it by z-index value, it gives us a "graphic" status of all the layers sorting, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then, maybe it would be nice to have the tabs prepared to be in the same line for all the values!
'modal-basic': 100,
'cookie-banner': 50,
'range-datepicker-icon': 2,
To have it visually more sorted! :D
@@ -7,3 +7,4 @@ | |||
@import './utils/text'; | |||
@import './utils/url'; | |||
@import './utils/colors'; | |||
//@import './utils/z-index'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment here!
WiP This fixes #83.