feat: create BaseButton component #1123

Merged
dpschen merged 3 commits from dpschen/frontend:feature/feat-BaseButton-component into main 2022-01-04 18:58:07 +00:00
4 changed files with 175 additions and 72 deletions
Showing only changes of commit 4d659be66a - Show all commits

View File

@ -1,5 +1,6 @@
<template>
<ready :class="{'is-touch': isTouch}">
<ready>
<div :class="{'is-touch': isTouch}">
<div :class="{'is-hidden': !online}">
<template v-if="authUser">
<top-navigation/>
@ -15,6 +16,7 @@
<transition name="fade">
<keyboard-shortcuts v-if="keyboardShortcutsActive"/>
</transition>
</div>
</ready>
</template>

View File

@ -0,0 +1,118 @@
<template>
<component
:is="componentNodeName"
class="base-button"
:class="{ 'base-button--type-button': isButton }"
v-bind="elementBindings"
:disabled="disabled || undefined"
>
<slot />
</component>
</template>
<script lang="ts">
// see https://v3.vuejs.org/api/sfc-script-setup.html#usage-alongside-normal-script
export default {
inheritAttrs: false,
}
</script>
<script lang="ts" setup>
// this component removes styling differences between links / vue-router links and button elements
// by doing so we make it easy abstract the functionality from style and enable easier and semantic
// correct button and link usage. Also see: https://css-tricks.com/a-complete-guide-to-links-and-buttons/#accessibility-considerations
// the component tries to heuristically determine what it should be checking the props (see the
// componentNodeName and elementBindings ref for this).
// NOTE: Do NOT use buttons with @click to push routes. => Use router-links instead!
import { ref, watchEffect, computed, useAttrs, PropType } from 'vue'
const BASE_BUTTON_TYPES_MAP = Object.freeze({
button: 'button',
submit: 'submit',
})
type BaseButtonTypes = keyof typeof BASE_BUTTON_TYPES_MAP
konrad marked this conversation as resolved Outdated

Why not a typescript string enum?

Why not a typescript string enum?

For this case: might make sense!

Why I still think we should keep it here:
In other components we might have a similar pattern where the values of the map are not strings but objects. E.g. think of a component that changes a configuration based on a prop where you just name the id of the configuration.

const CONFIGURATION_TYPES_MAP = Object.freeze({
	simple: { onlyOneProperty: 1 },
	complex: { here: 1, are: 2, many: 3, properties: 4 }
})

type ConfigurationTypes = keyof typeof CONFIGURATION_TYPES_MAP

AFAIK that would be not possible with enums

For this case: might make sense! Why I still think we should keep it here: In other components we might have a similar pattern where the values of the map are not strings but objects. E.g. think of a component that changes a configuration based on a prop where you just name the id of the configuration. ```ts const CONFIGURATION_TYPES_MAP = Object.freeze({ simple: { onlyOneProperty: 1 }, complex: { here: 1, are: 2, many: 3, properties: 4 } }) type ConfigurationTypes = keyof typeof CONFIGURATION_TYPES_MAP ``` AFAIK that would be not possible with enums

Couldn't you use interfaces with typescript enums for that use case?

Couldn't you use interfaces with typescript enums for that use case?

Not sure how you mean that?

An interface would just define the steucture of the configuration object, right?

So by doing this we would not recieve a configuration yet. What I want is the possibilty to chose between predefined configurations via a prop that provides an id (that selects one configuration).

Not sure how you mean that? An interface would just define the steucture of the configuration object, right? So by doing this we would not recieve a configuration yet. What I want is the possibilty to chose between predefined configurations via a prop that provides an id (that selects one configuration).

I had something like this in mind:

interface testIn {
	foo: string
}

enum test {
	one = testIn,
}

That does not work though, so looks like I was wrong. I took a look in the docs and it seems like you can only use numeric or string literals in enums.

Anyway, as you correctly pointed out that would not make sense to do what I had in mind. I think your use case is completely valid here :)

For consistency between the button and base button I think it should be fine to keep this here.

I had something like this in mind: ```ts interface testIn { foo: string } enum test { one = testIn, } ``` That does not work though, so looks like I was wrong. I took a look in the docs and it seems like you can only use numeric or string literals in enums. Anyway, as you correctly pointed out that would not make sense to do what I had in mind. I think your use case is completely valid here :) For consistency between the button and base button I think it should be fine to keep this here.
const props = defineProps({
type: {
type: String as PropType<BaseButtonTypes>,
default: 'button',
},
disabled: {
type: Boolean,
default: false,
},
})
const componentNodeName = ref<Node['nodeName']>('button')
interface ElementBindings {
type?: string;
rel?: string,
}
const elementBindings = ref({})
const attrs = useAttrs()
watchEffect(() => {
// by default this component is a button element with the attribute of the type "button" (default prop value)
let nodeName = 'button'
let bindings: ElementBindings = {type: props.type}
// if we find a "to" prop we set it as router-link
if ('to' in attrs) {
nodeName = 'router-link'
bindings = {}
}
// if there is a href we assume the user wants an external link via a link element
// we also set the attribute rel to "noopener" but make it possible to overwrite this by the user.
if ('href' in attrs) {
nodeName = 'a'
bindings = {rel: 'noopener'}
}
componentNodeName.value = nodeName
dpschen marked this conversation as resolved Outdated

Why the :where() instead of just using .base-button--type-button directly?

Why the `:where()` instead of just using `.base-button--type-button` directly?

By using :where as the main element class we make it possible to easily pverwrite the seit properties by a parent element. The reason beinh that the specifity of a class wrapped in :where is lower than a class selector in the parent. This way we have sane default styles that are still easy to overwrite (aka without stuff like id selectors, duplicate classes, element selectors or even !important.

By using `:where` as the main element class we make it possible to easily pverwrite the seit properties by a parent element. The reason beinh that the specifity of a class wrapped in `:where` is lower than a class selector in the parent. This way we have sane default styles that are still easy to overwrite (aka without stuff like id selectors, duplicate classes, element selectors or even `!important`.
elementBindings.value = {
...bindings,
...attrs,
}
})
const isButton = computed(() => componentNodeName.value === 'button')
</script>
<style lang="scss">
// NOTE: we do not use scoped styles to reduce specifity and make it easy to overwrite
// We reset the default styles of a button element to enable easier styling
:where(.base-button--type-button) {
border: 0;
margin: 0;
padding: 0;
text-decoration: none;
background-color: transparent;
text-align: center;
appearance: none;
}
:where(.base-button) {
cursor: pointer;
display: block;
color: inherit;
font: inherit;
user-select: none;
pointer-events: auto; // disable possible resets
&:focus {
outline: transparent;
}
&[disabled] {
cursor: default;
}
}
</style>

View File

@ -1,79 +1,64 @@
<template>
<a
<BaseButton
class="button"
:class="{
'is-loading': loading,
'has-no-shadow': !shadow,
'is-primary': type === 'primary',
'is-outlined': type === 'secondary',
'is-text is-inverted has-no-shadow underline-none':
type === 'tertary',
}"
:disabled="disabled || null"
@click="click"
:href="href !== '' ? href : null"
:class="[
variantClass,
{
'is-loading': loading,
'has-no-shadow': !shadow || variant === 'tertiary',
}
]"
>
<icon :icon="icon" v-if="showIconOnly"/>
<span class="icon is-small" v-else-if="icon !== ''">
<icon :icon="icon"/>
</span>
<slot></slot>
</a>
<slot />
</BaseButton>
</template>
<script>
<script lang="ts">
export default {
name: 'x-button',
props: {
type: {
type: String,
default: 'primary',
},
href: {
type: String,
default: '',
},
to: {
default: false,
},
icon: {
default: '',
},
loading: {
type: Boolean,
default: false,
},
shadow: {
type: Boolean,
default: true,
},
disabled: {
type: Boolean,
default: false,
},
},
emits: ['click'],
computed: {
showIconOnly() {
return this.icon !== '' && typeof this.$slots.default === 'undefined'
},
},
methods: {
click(e) {
if (this.disabled) {
return
}
if (this.to !== false) {
this.$router.push(this.to)
}
this.$emit('click', e)
},
},
}
</script>
<script setup lang="ts">
import {computed, useSlots, PropType} from 'vue'
import BaseButton from '@/components/base/BaseButton.vue'
const BUTTON_TYPES_MAP = Object.freeze({
primary: 'is-primary',
secondary: 'is-outlined',
tertiary: 'is-text is-inverted underline-none',
})
type ButtonTypes = keyof typeof BUTTON_TYPES_MAP
const props = defineProps({
variant: {
type: String as PropType<ButtonTypes>,
default: 'primary',
},
icon: {
default: '',
},
loading: {
type: Boolean,
default: false,
},
shadow: {
type: Boolean,
default: true,
},
})
const variantClass = computed(() => BUTTON_TYPES_MAP[props.variant])
const slots = useSlots()
const showIconOnly = computed(() => props.icon !== '' && typeof slots.default === 'undefined')
</script>
<style lang="scss" scoped>
.button {
transition: all $transition;
@ -83,8 +68,8 @@ export default {
font-weight: bold;
height: $button-height;
box-shadow: var(--shadow-sm);
display: inline-flex;
&.is-hovered,
&:hover {
box-shadow: var(--shadow-md);
}
@ -106,9 +91,10 @@ export default {
color: var(--white);
}
&.is-small {
border-radius: $radius;
}
}
.is-small {
border-radius: $radius;
}
.underline-none {

View File

@ -1,11 +1,8 @@
import {createApp, configureCompat} from 'vue'
// default everything to Vue 3 behavior
configureCompat({
dpschen marked this conversation as resolved Outdated

Is it still required to configure this here even though it is already set in vite's config?

Is it still required to configure this here even though it is already set in vite's config?

Would make sense.

I followed this guide (see step 3 under vite).

And then in the same guide under Global Confuguration they tell you to use this ConfuigureCompat method. This is why I made that error.

I guess the property in the vite config is alternative.
The location in vite.config seems better fitted since it's build related, so will remove here :)

Would make sense. I followed [this guide](https://v3.vuejs.org/guide/migration/migration-build.html#preparations) (see step 3 under vite). And then in the same guide under [Global Confuguration](https://v3.vuejs.org/guide/migration/migration-build.html#installation) they tell you to use this ConfuigureCompat method. This is why I made that error. I guess the property in the vite config is alternative. The location in vite.config seems better fitted since it's build related, so will remove here :)
COMPONENT_V_MODEL: false,
COMPONENT_ASYNC: false,
RENDER_FUNCTION: false,
WATCH_ARRAY: false, // TODO: check this again; this might lead to some problemes
TRANSITION_GROUP_ROOT: false,
MODE: 3,
})
import App from './App.vue'