feat(toggle-buttons-group): new-design (#1141)

* 1140-feat(toggle-buttons-group):new-design * class name change * update test md * Clear DOM * add the relevant test Only selected should have `filled` added to it (test fails because it is a new API) * Remove all buttons filled rule This will make current tests to fail * Remove obsolete API * Discard some irrelevant "filled" lines in the tests * Make all tests pass * select & disable mode fix * select & disable mode fix * remove accent from storybook * Remove filled from unselected * Refactor test * Add coverage for case of switching selection in regards to filled state * story adjustment Co-authored-by: yinon <yinon@hotmail.com> Co-authored-by: yonatankra <yonatan.kra@vonage.com>
Vonage · Dec 20, 2021 · 9b2a75c · 9b2a75c
1 parent 4004548
commit 9b2a75c
Show file tree

Hide file tree

Showing 8 changed files with 348 additions and 207 deletions.
diff --git a/__snapshots__/Toggle-buttons-group.md b/__snapshots__/Toggle-buttons-group.md
@@ -3,8 +3,10 @@
 #### `should have internal contents`
 
 ```html
-<slot>
-</slot>
+<div class="button-toggle-group">
+  <slot>
+  </slot>
+</div>
 
 ```
 
diff --git a/components/button-toggle-group/readme.md b/components/button-toggle-group/readme.md
@@ -36,7 +36,6 @@
 |values|property|[]string|The values of the selected items. Setting this property sets the state according to the items with the values in the array|
 |items|property|Element[]|A list of valid toggle elements|
 |shape|reflected|“pill”/”rounded”||
-|accent|reflected|boolean||
 |dense|reflected|boolean|Set the size of the buttons as dense|
 |enlarged|reflected|boolean|Set the size of the buttons as enlarged|
 |required|reflected|boolean|Prevents toggling off the last selection. Also does not send event of non toggled selection.|

diff --git a/components/button-toggle-group/src/vwc-button-toggle-group.scss b/components/button-toggle-group/src/vwc-button-toggle-group.scss
@@ -20,98 +20,55 @@ $toggle-buttons-type-selectors: selector.append('vwc-', list.join($scopes, '', $
 );
 
 $sizes: (
-	'normal': 40px,
-	'enlarged': 48px,
-	'dense': 32px,
+	'normal': 32px,
+	'enlarged': 40px,
+	'dense': 24px,
 );
 
-:host {
-	display: flex;
-}
 
 ::slotted(#{vwc-button}) {
 	--mdc-ripple-focus-opacity: 0;
 }
 
+.button-toggle-group {
+	display: inline-flex;
+	align-items: center;
+	padding: 3px 2px;
+	border: 1px solid var(#{scheme-variables.$vvd-color-neutral-30});
+	border-radius: var(#{$vvd-toggle-button-group-shape});
+
+	:host([shape='pill']) & {
+		--vvd-shape: var(#{$vvd-toggle-button-group-shape});
+	}
+}
+
 @each $scope in $scopes {
 	::slotted(vwc-#{$scope}) {
-		--vvd-#{$scope}-color: var(#{scheme-variables.$vvd-color-neutral-30});
-		--vvd-#{$scope}-on-color: var(#{scheme-variables.$vvd-color-on-canvas});
 		--vvd-#{$scope}-shape: var(#{$vvd-toggle-button-group-shape});
+		margin: 0 2px;
 	}
 
 	::slotted(vwc-#{$scope}[selected]) {
-		--vvd-#{$scope}-color: var(#{scheme-variables.$vvd-color-neutral-50});
-	}
-
-	:host([accent]) {
-
-		--vwc-#{$scope}-vertical-padding: 2px;
-		--vwc-#{$scope}-horizontal-padding: 1px;
-		--vvd-#{$scope}-block-size: calc(#{map.get($sizes, 'normal')} - 2 * var(--vwc-#{$scope}-vertical-padding));
-
-		::slotted(vwc-#{$scope}) {
-			padding: var(--vwc-#{$scope}-vertical-padding, 0) var(--vwc-#{$scope}-horizontal-padding, 0);
-			background-color: var(#{scheme-variables.$vvd-color-neutral-30});
-		}
-
-		::slotted(vwc-#{$scope}[selected], vwc-#{$scope}:hover:not([selected])) {
-			--vvd-#{$scope}-shape: var(#{$vvd-toggle-button-group-shape});
-		}
-
-		::slotted(vwc-#{$scope}[selected]) {
-			--vvd-#{$scope}-color: var(#{scheme-variables.$vvd-color-primary});
-			--vvd-#{$scope}-on-color: var(#{scheme-variables.$vvd-color-on-primary});
-		}
-
-		::slotted(vwc-#{$scope}:hover:not([selected])) {
-			// --vvd-#{$scope}-layout--fill: var(#{scheme-variables.$vvd-color-neutral-50});
-		}
+		--vvd-#{$scope}-color: var(#{scheme-variables.$vvd-color-primary});
+		--vvd-#{$scope}-on-color: var(#{scheme-variables.$vvd-color-on-primary});
 	}
 
-	:host([accent][dense]) {
-		--vvd-#{$scope}-block-size: calc(#{map.get($sizes, 'dense')} - 2 * var(--vwc-#{$scope}-vertical-padding));
+	:host {
+		--vvd-#{$scope}-block-size: calc(#{map.get($sizes, 'normal')});
 	}
 
-	:host([accent][enlarged]) {
-		--vvd-#{$scope}-block-size: calc(#{map.get($sizes, 'enlarged')} - 2 * var(--vwc-#{$scope}-vertical-padding));
+	:host([dense]) {
+		--vvd-#{$scope}-block-size: calc(#{map.get($sizes, 'dense')});
 	}
 
-	::slotted(vwc-#{$scope}:hover:not([selected])) {
-		//! @yonatankra is this mandatory?
-		//! we are good with using ripple for elevation indication in other components
-		// --vvd-#{$scope}-layout--fill: var(#{scheme-variables.$vvd-color-neutral-40});
+	:host([enlarged]) {
+		--vvd-#{$scope}-block-size: #{map.get($sizes, 'enlarged')};
 	}
 
-	::slotted(vwc-#{$scope}:not(:first-child):not(:last-child)) {
-		:host(:not([accent])) & {
-			--vvd-#{$scope}-shape: 0;
-		}
-	}
-
-	::slotted(vwc-#{$scope}:first-child:not(:last-child)) {
-		padding-left: var(--vwc-#{$scope}-vertical-padding, 0);
-		border-radius: var(#{$vvd-toggle-button-group-shape}) 0 0 var(#{$vvd-toggle-button-group-shape});
-
-		:host(:not([accent])) & {
-			--vvd-#{$scope}-shape: var(#{$vvd-toggle-button-group-shape}) 0 0 var(#{$vvd-toggle-button-group-shape});
-		}
-	}
-
-	::slotted(vwc-#{$scope}:last-child:not(:first-child)) {
-		padding-right: var(--vwc-#{$scope}-vertical-padding, 0);
-		border-radius: 0 var(#{$vvd-toggle-button-group-shape}) var(#{$vvd-toggle-button-group-shape}) 0;
-
-		:host(:not([accent])) & {
-			--vvd-#{$scope}-shape: 0 var(#{$vvd-toggle-button-group-shape}) var(#{$vvd-toggle-button-group-shape}) 0;
-		}
-	}
-
-	:host([disabled]) ::slotted(vwc-#{$scope}), :host([disabled][accent]) ::slotted(vwc-#{$scope}[selected]) {
-		//! @yonatankra not sure the following variables are required
-		// --vvd-#{$scope}-layout--fill: var(#{scheme-variables.$vvd-color-neutral-30});
-		// --vvd-#{$scope}-layout--text: var(#{scheme-variables.$vvd-color-neutral-50});
+	:host([disabled]) ::slotted(vwc-#{$scope}),
+	:host([disabled]) ::slotted(vwc-#{$scope}[selected]) {
 		pointer-events: none;
 	}
-
 }
+
+
diff --git a/components/button-toggle-group/src/vwc-button-toggle-group.ts b/components/button-toggle-group/src/vwc-button-toggle-group.ts
@@ -24,14 +24,6 @@ function isButtonActive(buttonElement: Element) {
 	return buttonElement.hasAttribute(SELECTED_ATTRIBUTE_NAME);
 }
 
-function toggleButton(buttonElement: Element) {
-	if (isButtonActive(buttonElement)) {
-		buttonElement.removeAttribute(SELECTED_ATTRIBUTE_NAME);
-	} else {
-		buttonElement.setAttribute(SELECTED_ATTRIBUTE_NAME, 'true');
-	}
-}
-
 @customElement('vwc-button-toggle-group')
 export class VWCButtonToggleGroup extends LitElement {
 	/**
@@ -98,9 +90,7 @@ export class VWCButtonToggleGroup extends LitElement {
 			values = [values[0]];
 		}
 		this.items.forEach((child) => {
-			values.includes(child.getAttribute('value')) ?
-				child.setAttribute(SELECTED_ATTRIBUTE_NAME, '') :
-				child.removeAttribute(SELECTED_ATTRIBUTE_NAME);
+			this.toggleButtonSelectedState(child, values.includes(child.getAttribute('value')));
 		});
 	}
 
@@ -146,7 +136,7 @@ export class VWCButtonToggleGroup extends LitElement {
 
 	protected override render(): unknown {
 		return html`
-			<slot></slot>`;
+			<div class="button-toggle-group"><slot></slot></div>`;
 	}
 
 	private toggleChildrenDisabledState() {
@@ -181,16 +171,26 @@ export class VWCButtonToggleGroup extends LitElement {
 			this.clearSelection(buttonElement);
 		}
 		if (this.isButtonValidForToggle(buttonElement)) {
-			toggleButton(buttonElement);
+			this.toggleButtonSelectedState(buttonElement, !isButtonActive(buttonElement));
 			this.dispatchToggleEvent();
 		}
 	}
 
 	private setNodeAttributes(buttonElement: Element) {
-		buttonElement.setAttribute('layout', 'filled');
+		this.toggleButtonSelectedState(buttonElement, isButtonActive(buttonElement));
 		this.toggleChildDisabledState(buttonElement);
 	}
 
+	private toggleButtonSelectedState(buttonElement: Element, isSelected: boolean) {
+		if (isSelected) {
+			buttonElement.setAttribute('layout', 'filled');
+			buttonElement.setAttribute(SELECTED_ATTRIBUTE_NAME, '');
+		} else {
+			buttonElement.removeAttribute(SELECTED_ATTRIBUTE_NAME);
+			buttonElement.removeAttribute('layout');
+		}
+	}
+
 	private setVwcButtonSize(buttonElement: Element) {
 		const size = this.size;
 
@@ -213,7 +213,7 @@ export class VWCButtonToggleGroup extends LitElement {
 	private clearSelection(buttonElement?: Element) {
 		this.items.forEach((button) => {
 			if (button === buttonElement) return;
-			button.removeAttribute(SELECTED_ATTRIBUTE_NAME);
+			this.toggleButtonSelectedState(button, false);
 		});
 	}