Fix SCSS Warnings #534
Fix SCSS Warnings #534
Conversation
MatthewKennedy
commented
Dec 12, 2022
•
MatthewKennedy
commented
- Add StyleLint
- Clean up SCSS
- Fix any warnings
- Add Github Workflow to run Style Lint
- Removed 2 reported unrecognised css properties
| @@ -97,6 +96,5 @@ | |||
| overflow-y: auto; | |||
| overflow-x: hidden; | |||
| max-height: $select-max-height-dropdown; | |||
| overflow-scrolling: touch; | |||
There was a problem hiding this comment.
Unrecognised style, this might want putting back in and the style lint rule silencing.
It might get picked up by postCSS down the line and auto-prefixer might add --webkit
There was a problem hiding this comment.
When the build CSS is compiled this is not prefixed with --webkit-
| $select-color-text: gray("800") !default; //$gray-800 | ||
| $select-color-highlight: rgba(255,237,40,0.4) !default; | ||
| $select-color-text: gray("800") !default; // $gray-800 | ||
| $select-color-highlight: rgb(255 237 40 / 40%) !default; |
There was a problem hiding this comment.
Double check that these modern colors work as expected, if not silence the style lint rule and convert back to classic.
There was a problem hiding this comment.
These colors compile down to standard colors okay in the build output CSS.
src/scss/tom-select.bootstrap4.scss
Outdated
| $select-color-optgroup: $dropdown-bg !default; | ||
| $select-color-optgroup-text: $dropdown-header-color !default; | ||
| $select-color-optgroup-border: $dropdown-divider-bg !default; | ||
| $select-color-dropdown: $dropdown-bg !default; | ||
| $select-color-dropdown-border-top: mix($input-border-color, $input-bg, 0.8) !default; | ||
| $select-color-dropdown-border-top: mix($input-border-color, $input-bg, 0.8%) !default; |
There was a problem hiding this comment.
Fix for latest SASS release depreciation warning.
There was a problem hiding this comment.
This should be 80% and not 0.8% right? https://github.com/orchidjs/tom-select/pull/529/files
| @@ -159,7 +150,6 @@ $select-spinner-border-color: $select-color-border !default; | |||
| font-family: $select-font-family; | |||
| font-size: $select-font-size; | |||
| line-height: $select-line-height; | |||
| font-smoothing: $select-font-smoothing; | |||
There was a problem hiding this comment.
Another unrecognised property, and its accompanying SCSS var, removed, might want putting back in?
There was a problem hiding this comment.
When the build CSS is compiled this is not prefixed with --webkit-
| display:flex; | ||
| align-items: center; | ||
|
|
||
| &.dropdown -active { |
There was a problem hiding this comment.
This was removed, because the SCSS as it was did nothing &.dropdown -active is not valid, and looking at the html, I can't see any instance of a .dropdown-active inside a .ts-control
|
Ok, there's a lot to look over here. In the future, it would be better to submit multiple pull requests. |
|
Sorry about that, got a bit carried away. |
src/scss/tom-select.bootstrap4.scss
Outdated
|
|
||
| // box-shadow: none; |
There was a problem hiding this comment.
Linting is adding a space before comments, maybe these comments are not even necessary here ...
| // box-shadow: none; |
There was a problem hiding this comment.
Probably not, but I didn't write the original code so maybe it was a reminder for something, I'll remove those comments.
src/scss/tom-select.bootstrap5.scss
Outdated
| $select-color-optgroup: $dropdown-bg !default; | ||
| $select-color-optgroup-text: $dropdown-header-color !default; | ||
| $select-color-optgroup-border: $dropdown-divider-bg !default; | ||
| $select-color-dropdown: $dropdown-bg !default; | ||
| $select-color-dropdown-border-top: mix($input-border-color, $input-bg, 0.8) !default; | ||
| $select-color-dropdown-border-top: mix($input-border-color, $input-bg, 0.8%) !default; |
There was a problem hiding this comment.
| $select-color-dropdown-border-top: mix($input-border-color, $input-bg, 0.8%) !default; | |
| $select-color-dropdown-border-top: mix($input-border-color, $input-bg, 80%) !default; |
Isn't this one also 80%?
There was a problem hiding this comment.
I'll look at this and get back to you this afternoon.
src/scss/tom-select.scss
Outdated
| $select-ns: 'ts' !default; | ||
| $select-font-family: inherit !default; | ||
| $select-font-size: 13px !default; | ||
| $select-line-height: 18px !default; | ||
| $select-color-text: #303030 !default; | ||
| $select-color-border: #d0d0d0 !default; | ||
| $select-color-highlight: rgb(125 168 208 / 20%) !default; | ||
| $select-color-input: #fff !default; | ||
| $select-color-input-full: $select-color-input !default; | ||
| $select-color-disabled: #fafafa !default; | ||
| $select-color-item: #f2f2f2 !default; | ||
| $select-color-item-text: $select-color-text !default; | ||
| $select-color-item-border: #d0d0d0 !default; | ||
| $select-color-item-active: #e8e8e8 !default; | ||
| $select-color-item-active-text: $select-color-text !default; | ||
| $select-color-item-active-border: #cacaca !default; | ||
| $select-color-dropdown: #fff !default; | ||
| $select-color-dropdown-border: $select-color-border !default; | ||
| $select-color-dropdown-border-top: #f0f0f0 !default; | ||
| $select-color-dropdown-item-active: #f5fafd !default; | ||
| $select-color-dropdown-item-active-text: #495c68 !default; | ||
| $select-color-dropdown-item-create-text: rgba(red($select-color-text), green($select-color-text), blue($select-color-text), 50%) !default; |
There was a problem hiding this comment.
This doesn't look to be aligned, how does the lint let them through?
Maybe there should be no tabs, only always a space, between attributes and values attribute: value
There was a problem hiding this comment.
They line up in the editor okay, but converted tabs to spaces.
There was a problem hiding this comment.
FYI: Here's the editorconfig file for tom-select. I know there are arguments for both tabs and spaces, but I prefer tabs.
|
It should be great to test and merge this pull request. |
I agree, but I see that this repository had the last merged pull request last December. |
@oyejorge Are you still willing/planning to maintain tom-select or is it getting abandoned? |
|
https://www.npmjs.com/package/@remotedevforce/tom-select implements this solution and also solves some other issues if you wish to try it. |
