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.

Sign up for GitHub

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

fix(MultiSelect): Apply a temporary fix for Vanilla includes in MultiSelect component #1052

Merged
merged 3 commits into from
Mar 19, 2024
Merged

fix(MultiSelect): Apply a temporary fix for Vanilla includes in MultiSelect component #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f0405b5
Update Vanilla to 4.9.0
bartaz Mar 14, 2024
a1b69bc
Remove Vanilla extends from MultiSelect.scss to fix duplicated form s…
bartaz Mar 14, 2024
262d317
Scope Vanilla form include to avoid conflicts with Vanilla base
bartaz Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"ts-jest": "29.1.2",
"tsc-alias": "1.8.8",
"typescript": "5.3.3",
"vanilla-framework": "4.6.0",
"vanilla-framework": "4.9.0",
"wait-on": "7.2.0",
"webpack": "5.89.0"
},
Expand Down
5 changes: 1 addition & 4 deletions src/components/Icon/Icon.stories.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
@import "~vanilla-framework/scss/settings";
@import "~vanilla-framework/scss/base_placeholders";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@import "~vanilla-framework/scss/base_icon-definitions";
@import "~vanilla-framework/scss/patterns_icons";
@import "vanilla-framework";

@include vf-b-placeholders;
@include vf-p-icons-common;
Expand Down
35 changes: 17 additions & 18 deletions src/components/MultiSelect/MultiSelect.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is safe to include, as it only contains placeholders, not any actual Vanilla styling.


$dropdown-max-height: 20rem;

.multi-select {
// 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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petermakowski I think I found a cleaner workaround for extending Vanilla styles in components.


position: relative;
}

Expand Down Expand Up @@ -37,12 +41,12 @@ $dropdown-max-height: 20rem;
}

.multi-select__dropdown {
@extend %vf-bg--x-light;
@extend %vf-has-box-shadow;
background-color: $colors--theme--background-default;
box-shadow: $box-shadow;
color: $colors--theme--text-default;
left: 0;
max-height: $dropdown-max-height;
overflow: auto;

padding-top: $spv--small;
position: absolute;
right: 0;
Expand All @@ -59,14 +63,15 @@ $dropdown-max-height: 20rem;
}

.multi-select__dropdown-list {
@extend %vf-list;

list-style: none;
margin-bottom: $sph--x-small;
margin-left: 0;
padding-left: 0;
}

.multi-select__footer {
background: white;
border-top: 1px solid $color-mid-light;
background: $colors--theme--background-default;
border-top: 1px solid $colors--theme--border-default;
bottom: 0;
display: flex;
flex-wrap: wrap;
Expand Down Expand Up @@ -97,12 +102,6 @@ $dropdown-max-height: 20rem;
}
}

.multi-select__dropdown-item-description {
bartaz marked this conversation as resolved.
Show resolved Hide resolved
@extend %small-text;

color: $color-mid-dark;
}

.multi-select__dropdown-button {
border: 0;
margin-bottom: 0;
Expand All @@ -113,7 +112,7 @@ $dropdown-max-height: 20rem;
}

.multi-select__selected-list {
background-color: $colors--light-theme--background-inputs;
background-color: $colors--theme--background-inputs;
border-bottom: 0;
margin: 0;
overflow: hidden;
Expand Down Expand Up @@ -142,12 +141,12 @@ $dropdown-max-height: 20rem;
transform: translateY(-50%) rotate(-180deg);

@extend %icon;
@include vf-icon-chevron($color-mid-dark);
@include vf-icon-chevron-themed;
@include vf-transition($property: transform, $duration: fast);
}

&[aria-expanded="true"] {
background-color: $colors--light-theme--background-hover;
background-color: $colors--theme--background-hover;
}

&[aria-expanded="false"] {
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14500,10 +14500,10 @@ validate-npm-package-name@^5.0.0:
dependencies:
builtins "^5.0.0"

vanilla-framework@4.6.0:
version "4.6.0"
resolved "https://registry.yarnpkg.com/vanilla-framework/-/vanilla-framework-4.6.0.tgz#61b93a2197eed0d869f85e1014dd9e67bacc355e"
integrity sha512-pRjJknqsL4CLA+ovLMlg7MhVh9nIcu7Ev81Z62gh3hAzUDIiZOUI2W9+B16XWv4QFmFxEOgQcfVi2cezLun9SQ==
vanilla-framework@4.9.0:
version "4.9.0"
resolved "https://registry.yarnpkg.com/vanilla-framework/-/vanilla-framework-4.9.0.tgz#7566b42a22c2394ea1d7ea843d24ec305446cb3e"
integrity sha512-iTmvqWlsX0ic69VZ1sR9NPQtYRR9+iM679HZCl7SDQhMQSsEeJEQ6Ejjen3JN5a7YxiYmeIhxaLlmC4rExRT1w==

vary@~1.1.2:
version "1.1.2"
Expand Down
Loading