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

Jump to bottom

ToggleButton and ToggleButtonGroup #358

Merged
merged 10 commits into from
Jan 26, 2021
Merged

ToggleButton and ToggleButtonGroup #358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
354f3a1
ToggleButton PoC
micgro42 Jan 19, 2021
598982a
Second attempt: fix it in render
micgro42 Jan 19, 2021
3a81222
Silencing typescript errors
micgro42 Jan 19, 2021
12102a0
Use provide/inject
micgro42 Jan 20, 2021
f74267a
Fixed injection types
micgro42 Jan 20, 2021
1205f30
Add documentation to props and slots
micgro42 Jan 20, 2021
c58ff79
Make ToggleButton ready for review
Ladsgroup Jan 22, 2021
544b1b2
Fixes
Ladsgroup Jan 26, 2021
97579c8
Update vue-components/src/components/ToggleButton.vue
Ladsgroup Jan 26, 2021
deaf265
Update vue-components/src/components/ToggleButton.vue
Ladsgroup Jan 26, 2021
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
123 changes: 123 additions & 0 deletions vue-components/src/components/ToggleButton.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
<template>
<button
:class="['wikit', 'wikit-ToggleButton', buttonIsActive ? 'wikit-ToggleButton--isActive' : null]"
@click="onClick"
>
<!-- @slot required. add the content of the button here (label, icon, etc.) -->
<slot />
</button>
</template>

<script lang="ts">
import Vue, { VueConstructor } from 'vue';
import { ToggleButtonGroupInjection } from '@/components/ToggleButtonGroup.vue';

export default ( Vue as VueConstructor<Vue & ToggleButtonGroupInjection> ).extend( {
name: 'ToggleButton',
methods: {
onClick(): void {
if ( this.toggleListener !== null ) {
this.toggleListener( this.value );
return;
}
/**
* only emitted when not use as part of a ToggleButtonGroup
*/
this.$emit( 'click' );
},
},
inject: {
groupValue: { default: null },
toggleListener: { default: null },
} as Record<keyof ToggleButtonGroupInjection, object>,
computed: {
buttonIsActive(): boolean {
if ( this.groupValue !== null ) {
return this.groupValue() === this.value;
}
return this.isActive;
},
},
props: {
/**
* required when the ToggleButton is used as part of a ToggleButtonGroup.
*/
value: {
default: null,
type: String,
},
/**
* required when the ToggleButton is used as a standalone component.
* Ignored when used as part of a ToggleButtonGroup
*/
isActive: {
default: false,
type: Boolean,
},
},
} );
</script>

<style lang="scss">
.wikit-ToggleButton {
color: $wikit-ToggleButton-color;
font-family: $wikit-ToggleButton-font-family;
font-weight: $wikit-ToggleButton-font-weight;
font-size: $wikit-ToggleButton-font-size;
line-height: $wikit-ToggleButton-line-height;
box-sizing: border-box;
border-width: $wikit-ToggleButton-border-width;
border-style: $wikit-ToggleButton-border-style;
border-radius: $wikit-ToggleButton-border-radius;
border-color: $wikit-ToggleButton-border-color;
background-color: $wikit-ToggleButton-background-color;
padding-inline: $wikit-ToggleButton-medium-padding-horizontal;
padding-block: $wikit-ToggleButton-medium-padding-vertical;
transition-property: $wikit-ToggleButton-transition-property;
transition-duration: $wikit-ToggleButton-transition-duration;
transition-timing-function: $wikit-ToggleButton-transition-timing-function;

&:hover {
color: $wikit-ToggleButton-hover-color;
border-color: $wikit-ToggleButton-hover-border-color;
background-color: $wikit-ToggleButton-hover-background-color;
}

&:active {
color: $wikit-ToggleButton-active-color;
border-color: $wikit-ToggleButton-active-border-color;
background-color: $wikit-ToggleButton-active-background-color;
}

&:focus {
color: $wikit-ToggleButton-focus-color;
border-color: $wikit-ToggleButton-focus-border-color;
background-color: $wikit-ToggleButton-focus-background-color;
box-shadow: $wikit-ToggleButton-focus-box-shadow;
Ladsgroup marked this conversation as resolved.
Show resolved Hide resolved
outline: none;
}

&:disabled {
color: $wikit-ToggleButton-disabled-color;
border-color: $wikit-ToggleButton-disabled-border-color;
background-color: $wikit-ToggleButton-disabled-background-color;
}

@media (max-width: $width-breakpoint-mobile) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looking at storybook, I have the strong suspicion that this media-query is not working

Copy link
Contributor

Choose a reason for hiding this comment

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

I double checked it before making the patch and it worked fine. Maybe you're checking against tablet breakpoint? The mobile breakpoint is pretty small.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My bad, the media-query is indeed working fine!
It is that box-sizing: border-box; is missing. That is why the buttons are so strangely large ^^

The m-sized buttons are supposed to have a total height of 32px, not 44px :)

Also, @SaiSan-WMDE , I think we might be missing some component tokens for the min-height.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added box-sizing: border-box;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi! Thanks for working on this, and for adding box-sizing: border-box;. The size of buttons looks good now. We do have min-height tokens, but we haven't been using them with the button component because they proved to be unnecessary. Should that change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Quick question: In they proved to be unnecessary, by they you mean the min-height css style or the min-height tokens?
The min-height css styles are in the figma specs and I set them as it's there. If we need to not use them, that's fine.

If that's about the tokens themselves, I'm fine with moving like this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Using the CSS style, and therefore the tokens to define it, seemed to not have a real impact on the component, as far as I remember. I'd say we can move on like this.

I should be rephrasing many of the Figma specs, to avoid them from being interpreted as implementation decisions. Now that UX creates component tokens, these should be considered the design source of truth by developers.

(Leaving more feedback in a separate comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Done. Removed the min-height css.

padding-inline: $wikit-ToggleButton-large-padding-horizontal;
padding-block: $wikit-ToggleButton-large-padding-vertical;
}
}

.wikit-ToggleButton.wikit-ToggleButton--isActive {
color: $wikit-ToggleButton-selected-color;
border-color: $wikit-ToggleButton-selected-border-color;
background-color: $wikit-ToggleButton-selected-background-color;

&:focus {
box-shadow: $wikit-ToggleButton-selected-focus-box-shadow;
Ladsgroup marked this conversation as resolved.
Show resolved Hide resolved
outline: none;
}
}

</style>
84 changes: 84 additions & 0 deletions vue-components/src/components/ToggleButtonGroup.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<template>
<span class="wikit wikit-ToggleButtonGroup">
<slot />
</span>
</template>

<script lang="ts">
import Vue from 'vue';

export interface ToggleButtonGroupInjection {
groupValue: ( () => string ) | null;
toggleListener: ( ( value: string ) => void ) | null;
}

export default Vue.extend( {
name: 'ToggleButtonGroup',
provide(): ToggleButtonGroupInjection {
return {
groupValue: (): string => this.value,
toggleListener: ( event: string ): void => {
this.$emit( 'input', event );
},
};
},
props: {
value: {
required: true,
type: String,
},
},
} );
</script>
<style lang="scss">
.wikit-ToggleButtonGroup :not(:last-child) {
micgro42 marked this conversation as resolved.
Show resolved Hide resolved
border-inline-end: none;
border-start-end-radius: 0;
border-end-end-radius: 0;
}

.wikit-ToggleButtonGroup :not(:first-child) {
border-start-start-radius: 0;
border-end-start-radius: 0;
}

.wikit-ToggleButtonGroup :not(:first-child) {
border-start-start-radius: 0;
border-end-start-radius: 0;
}

.wikit-ToggleButtonGroup :disabled {
border-block: none;
}

.wikit-ToggleButtonGroup :focus {
outline: none;
}

.wikit-ToggleButtonGroup :not(:first-child):disabled {
border-inline-start-color: $color-base-100;
}

.wikit-ToggleButtonGroup {
[dir='ltr'] & :not(:first-child) {
border-top-left-radius: 0;
border-bottom-left-radius: 0;
}

[dir='rtl'] & :not(:first-child) {
border-top-right-radius: 0;
border-bottom-right-radius: 0;
}

[dir='ltr'] & :not(:last-child) {
border-top-right-radius: 0;
border-bottom-right-radius: 0;
}

[dir='rtl'] & :not(:last-child) {
border-top-left-radius: 0;
border-bottom-left-radius: 0;
}
}

</style>
4 changes: 4 additions & 0 deletions vue-components/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import Button from './components/Button.vue';
import TextInput from './components/TextInput.vue';
import ToggleButton from './components/ToggleButton.vue';
import ToggleButtonGroup from './components/ToggleButtonGroup.vue';
import Lookup from './components/Lookup.vue';
import Dropdown from './components/Dropdown.vue';
import Message from './components/Message.vue';
Expand All @@ -9,6 +11,8 @@ export {
Button,
Dropdown,
TextInput,
ToggleButton,
ToggleButtonGroup,
Lookup,
Message,
Icon,
Expand Down
127 changes: 127 additions & 0 deletions vue-components/stories/ToggleButton.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import ToggleButton from '@/components/ToggleButton';
import ToggleButtonGroup from '@/components/ToggleButtonGroup';
import { Component } from 'vue';

export default {
component: ToggleButton,
title: 'ToggleButton',
};
const options = [
{
label: 'Pineapple',
value: 'pineapple',
},
{
label: 'Mushrooms',
value: 'mushrooms',
},
{
label: 'Spinach',
value: 'spinach',
},
{
label: 'Artichoke',
value: 'artichoke',
},
];
export function Basic( args: { label: string } ): Component {
return {
components: { ToggleButton },
data(): unknown {
return {
isActive: false,
label: 'Single ToggleButton'
};
},
props: Object.keys( args ),
template: `
<div>
<ToggleButton
:is-active="isActive"
@click="() => isActive = !isActive"
>{{label}}</ToggleButton>
</div>
`,
};
}
Basic.args = {
label: 'Single ToggleButton',
};
Basic.argTypes = {
isActive: {
control: {
disable: true,
},
},
};
export function All(): Component {
return {
components: { ToggleButton },
data(): unknown {
return {
isActive: false,
};
},
template: `
<div>
<ToggleButton
:is-active="isActive"
@click="() => isActive = !isActive"
>Single Enabled ToggleButton</ToggleButton>
<br>
<br>
<ToggleButton
disabled
>Single Disabled ToggleButton</ToggleButton>
</div>
`,
};
}
export function Group(): Component {
return {
components: { ToggleButton, ToggleButtonGroup },
data(): unknown {
return {
selectedOption: '',
singleButtonIsActive: false,
};
},
computed: {
options() {
return options;
},
},
template: `
<div dir="ltr">
<div style="margin-top: 16px; font-family: sans-serif; color: #202122">
Best topping on Pizza:
<span v-if="selectedOption" class="selected-item-label">{{ selectedOption }}</span>
</div>
<br>
<ToggleButtonGroup
v-model="selectedOption"
>
<template v-slot:default>
<ToggleButton
v-for="option in options"
:value="option.value"
:key="option.value"
>{{ option.label }}</ToggleButton>
</template>
</ToggleButtonGroup>
<br>
<br>
<ToggleButtonGroup>
<template v-slot:default>
<ToggleButton
v-for="option in options"
:value="option.value"
:key="option.value"
disabled
>{{ option.label }}</ToggleButton>
</template>
</ToggleButtonGroup>
</div>
`,
};
}