fix(MultiSelect): Apply a temporary fix for Vanilla includes in MultiSelect component #1052
Conversation
|
Demo starting at https://react-components-1052.demos.haus |
Fix Vanilla import in icons custom CSS
bartaz
force-pushed
the
fix-multiselect-scss
branch
from
bbf9f54
to
a1b69bc
Compare
bartaz
changed the title
|
Percy: https://percy.io/bb49709b/react-components/builds/33166994/ The expected changes are visible (Slider is fixed). There are a lot of unrelated changes caused by Vanilla upgrade that includes new theming and new icon colours, etc. There is also some weird behaviour in Percy that it shows new input border as lighter than before, which is weird because when checked in the browser it doesn't look lighter than before. 
I'm ignoring it for now, assuming it's some Percy/Storybook glitch, but would be good to undestand the cause and if we can do anything about it, to avoid false negatives in future. |
| @@ -1,7 +1,4 @@ | |||
| @import "~vanilla-framework/scss/settings"; | |||
There was a problem hiding this comment.
Removing unnecessary individual imports.
It's all included in full Vanilla import and there is no downside to import it all (no CSS produced until anything is included).
|
|
||
| $dropdown-max-height: 20rem; | ||
|
|
||
| // XXX: Temporarly we are duplicating some of the placeholders defined in Vanilla |
There was a problem hiding this comment.
In longer term we should either upstream this component to Vanilla or refactor some bits of Vanilla to be able to extend these parts that are needed here, without the need to include the whole base or list styles, because duplication of base form styles was the cause of the issue (broken slider styles overriden by duplicated input styles).
There was a problem hiding this comment.
I'd suggest we link to a new issue in react-components where we explain this in detail (and move this comment there).
There was a problem hiding this comment.
It felt more fitting on Vanilla itself: canonical/vanilla-framework#5030
|
|
||
| $dropdown-max-height: 20rem; | ||
|
|
||
| // XXX: Temporarly we are duplicating some of the placeholders defined in Vanilla |
There was a problem hiding this comment.
I'd suggest we link to a new issue in react-components where we explain this in detail (and move this comment there).
petermakowski
added
Review: Code +1 (with changes)
Review: QA +1
and removed
Review: Code needed
Review: QA needed
labels
| @@ -1,11 +1,15 @@ | |||
| @use "sass:map"; | |||
| @import "vanilla-framework"; | |||
| @include vf-base; | |||
| @include vf-p-lists; | |||
| @include vf-b-placeholders; // Vanilla base placeholders to extend from | |||
There was a problem hiding this comment.
This is safe to include, as it only contains placeholders, not any actual Vanilla styling.
| // Scope Vanilla form includes to multi select component only | ||
| // to avoid overriding any Vanilla base styles | ||
| // XXX: This is a workaround for https://github.com/canonical/vanilla-framework/issues/5030 | ||
| @include vf-b-forms; |
There was a problem hiding this comment.
As for now vf-b-forms contains both placeholders that we want to extend here and default form styling (that we don't want to duplicate). So to avoid overriding default Vanilla we need to scope it within the component.
There was a problem hiding this comment.
@petermakowski I think I found a cleaner workaround for extending Vanilla styles in components.
|
🎉 This PR is included in version 0.50.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Done
@includes(vf-baseandvf-b-forms) and@extenduses fromMultiSelect.scssand replaces them with copies of relevant parts of Vanilla.This is a temporary solution until we find better ways to reuse bigger parts of Vanilla styling.
@extendusage by itself is not huge issue, but it does silently create a duplicate of Vanilla styling.Bigger problem was including
vf-baseandvf-lists(which contain required placeholders to extend) as they duplicate big chunks of Vanilla styling creating source order conflicts of old and new styles and breaking the styling of some other components.QA steps
Fixes
Fixes: #1041