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.

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(kfileupload, kselect, kmultiselect): kinput id [KHCP-12336] #2241

Merged
merged 11 commits into from
Jun 19, 2024
9 changes: 8 additions & 1 deletion sandbox/pages/SandboxFileUpload.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@
:links="inject('app-links', [])"
title="KFileUpload"
>
<div class="kfileupload-sandbox">
<KFileUpload
:accept="['image/*']"
label="Label"
/>
<div
v-if="false"
class="kfileupload-sandbox"
>
Leopoldthecoder marked this conversation as resolved.
Show resolved Hide resolved
<!-- Props -->
<SandboxTitleComponent
is-subtitle
Expand Down
9 changes: 8 additions & 1 deletion sandbox/pages/SandboxMultiselect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@
:links="inject('app-links', [])"
title="KMultiselect"
>
<div class="kmultiselect-sandbox">
<KMultiselect
:items="multiselectItems"
label="Label"
/>
<div
v-if="false"
class="kmultiselect-sandbox"
>
<!-- Props -->
<SandboxTitleComponent
is-subtitle
Expand Down
10 changes: 9 additions & 1 deletion sandbox/pages/SandboxSelect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@
:links="inject('app-links', [])"
title="KSelect"
>
<div class="kselect-sandbox">
<KSelect
id="foobar-here"
:items="selectItems"
label="Label"
/>
<div
v-if="false"
class="kselect-sandbox"
>
<!-- Examples -->
<SandboxTitleComponent
is-subtitle
Expand Down
64 changes: 53 additions & 11 deletions src/components/KFileUpload/KFileUpload.vue
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
<template>
<div class="k-file-upload">
<div
class="k-file-upload"
v-bind="modifiedAttrs"
>
<KLabel
v-if="label"
v-bind-once="{ for: inputId }"
v-bind="labelAttributes"
ref="labelElement"
:for="$attrs.id ? String($attrs.id) : undefined"
:required="isRequired"
>
{{ strippedLabel }}
Expand All @@ -26,9 +30,9 @@
</span>

<KInput
v-bind="attrs.id ? { id: String(attrs.id) } : {}"
:key="fileInputKey"
ref="fileInputElement"
v-bind-once="{ id: inputId }"
:accept="accept"
class="upload-input"
:disabled="disabled"
Expand Down Expand Up @@ -64,14 +68,19 @@
</div>
</template>

<script lang="ts">
export default {
inheritAttrs: false,
}
</script>

Leopoldthecoder marked this conversation as resolved.
Show resolved Hide resolved
<script lang="ts" setup>
import type { PropType } from 'vue'
import { computed, ref, useAttrs, useSlots } from 'vue'
import { computed, ref, useAttrs, useSlots, onMounted, watch, nextTick } from 'vue'
import KLabel from '@/components/KLabel/KLabel.vue'
import KInput from '@/components/KInput/KInput.vue'
import KButton from '@/components/KButton/KButton.vue'
import useUtilities from '@/composables/useUtilities'
import useUniqueId from '@/composables/useUniqueId'

const props = defineProps({
labelAttributes: {
Expand Down Expand Up @@ -127,8 +136,16 @@ const emit = defineEmits<{

const { stripRequiredLabel } = useUtilities()

const inputId = attrs.id ? String(attrs.id) : useUniqueId()
const modifiedAttrs = computed(() => {
const $attrs = { ...attrs }

delete $attrs.id // delete id because we bind id to the input element

return $attrs
})

const fileInputElement = ref<InstanceType<typeof KInput> | null>(null)
const labelElement = ref<InstanceType<typeof KLabel> | null>(null)
const hasLabelTooltip = computed((): boolean => !!(props.labelAttributes?.info || slots['label-tooltip']))
const strippedLabel = computed((): string => stripRequiredLabel(props.label, isRequired.value))
const isRequired = computed((): boolean => attrs?.required !== undefined && String(attrs?.required) !== 'false')
Expand Down Expand Up @@ -165,7 +182,7 @@ const hasUploadError = ref<boolean>(false)
// This holds the FileList
const fileInput = ref<File[]>([])
// To clear the input value after reset
const fileInputKey = ref(0)
const fileInputKey = ref<number>(0)
// File fakepath
const fileValue = ref<string>('')
// Array to store the previously selected FileList when user clicks reopen the file uploader and clicks on Cancel
Expand Down Expand Up @@ -235,6 +252,35 @@ const resetInput = (): void => {

emit('file-removed')
}

const setLabelAttributes = () => {
/**
* Temporary fix for the issue where we can't use v-bind-once to pass id to a custom element (KInput)
* TODO: remove this once useId is released in Vue 3.5
*/
if (!attrs.id) {
const inputElementId = fileInputElement.value?.$el?.querySelector('input')?.id

if (inputElementId) {
labelElement.value?.$el?.setAttribute('for', inputElementId)
}
}
}

watch(fileInputKey, async () => {
await nextTick()
Copy link
Member

Choose a reason for hiding this comment

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

Why does this code need to be in a separate watcher instead of in the one below that triggers the key to increment?

setLabelAttributes()
})

watch(() => attrs.id, async () => {
fileInputKey.value++
await nextTick()
setLabelAttributes()
portikM marked this conversation as resolved.
Show resolved Hide resolved
}, { immediate: true })

onMounted(() => {
setLabelAttributes()
})
</script>

<style lang="scss" scoped>
Expand All @@ -260,10 +306,6 @@ $kFileUploadInputPaddingY: var(--kui-space-40, $kui-space-40); // corresponds to
color: transparent !important;
}

:deep(.k-input) {
padding-right: 90px !important; // offset to account for button
}

.file-upload-input-wrapper {
position: relative;

Expand Down
2 changes: 1 addition & 1 deletion src/components/KInput/KInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ $kInputSlotSpacing: var(--kui-space-40, $kui-space-40); // $kSelectInputSlotSpac
width: $kInputIconSize !important;
}

:deep([role="button"]), :deep(button) {
:deep([role="button"]:not(.k-button)), :deep(button:not(.k-button)) {
@include defaultButtonReset;

color: var(--kui-color-text-neutral, $kui-color-text-neutral);
Expand Down
15 changes: 6 additions & 9 deletions src/components/KMultiselect/KMultiselect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
<div v-if="collapsedContext">
<KInput
ref="multiselectInputElement"
v-bind-once="{ id: multiselectId }"
autocapitalize="off"
autocomplete="off"
class="multiselect-input"
Expand Down Expand Up @@ -80,7 +79,7 @@
>
<KBadge
v-for="item, idx in visibleSelectedItems"
:key="`${multiselectId}-${item.key ? item.key : idx}-badge-${key}`"
:key="`${multiselectKey}-${item.key ? item.key : idx}-badge-${key}`"
:appearance="getBadgeAppearance(item)"
class="multiselect-selection-badge"
:icon-before="false"
Expand Down Expand Up @@ -158,9 +157,7 @@
class="multiselect-input-wrapper"
>
<KInput
v-bind="modifiedAttrs"
ref="multiselectDropdownInputElement"
v-bind-once="{ id: multiselectId }"
autocapitalize="off"
autocomplete="off"
class="multiselect-dropdown-input"
Expand Down Expand Up @@ -254,7 +251,7 @@
>
<KBadge
v-for="item, idx in visibleSelectedItemsStaging"
:key="`${multiselectId}-${item.key ? item.key : idx}-badge`"
:key="`${multiselectKey}-${item.key ? item.key : idx}-badge`"
aria-hidden="true"
class="multiselect-selection-badge"
:icon-before="false"
Expand Down Expand Up @@ -478,11 +475,11 @@ const defaultKPopAttributes = {
const key = ref(0)
const stagingKey = ref(0)

const multiselectWrapperId = useUniqueId() // unique id for the KPop target
const multiselectId = attrs.id ? String(attrs.id) : useUniqueId()
const multiselectWrapperId = attrs.id ? String(attrs.id) : useUniqueId() // unique id for the KLabel `for` attribute
const multiselectKey = useUniqueId()

const multiselectElement = ref<HTMLDivElement | null>(null)
const multiselectInputElement = ref<HTMLDivElement | null>(null)
const multiselectInputElement = ref<InstanceType<typeof KInput> | null>(null)
const multiselectDropdownInputElement = ref<HTMLDivElement | null>(null)
const multiselectSelectionsStagingElement = ref<HTMLDivElement>()

Expand Down Expand Up @@ -635,7 +632,7 @@ const handleToggle = async (open: boolean, isToggled: Ref<boolean>, toggle: () =

await nextTick()

const input = document?.getElementById(multiselectId) as HTMLInputElement
const input = multiselectInputElement.value?.$el.querySelector('input') as HTMLInputElement
input?.focus({ preventScroll: true })
}
} else {
Expand Down
35 changes: 31 additions & 4 deletions src/components/KSelect/KSelect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
>
<KLabel
v-if="label"
v-bind-once="{ for: selectId }"
ref="labelElement"
v-bind="labelAttributes"
data-testid="select-label"
:for="$attrs.id ? String($attrs.id) : undefined"
:required="isRequired"
>
{{ strippedLabel }}
Expand Down Expand Up @@ -39,7 +40,8 @@
@click="onSelectWrapperClick"
>
<KInput
v-bind-once="{ id: selectId }"
:key="inputKey"
ref="inputElement"
autocapitalize="off"
autocomplete="off"
class="select-input"
Expand All @@ -52,7 +54,7 @@
:model-value="filterQuery"
:placeholder="selectedItem && !enableFiltering ? selectedItem.label : placeholderText"
:readonly="isReadonly"
v-bind="modifiedAttrs"
v-bind="attrs.id ? { id: String(attrs.id), ...modifiedAttrs } : { ...modifiedAttrs }"
@blur="onInputBlur"
@focus="onInputFocus"
@keypress="onInputKeypress"
Expand Down Expand Up @@ -363,6 +365,10 @@ const defaultKPopAttributes = {
hideCaret: true,
}

const inputKey = ref<number>(0)
const inputElement = ref<InstanceType<typeof KInput> | null>(null)
const labelElement = ref<InstanceType<typeof KLabel> | null>(null)

const strippedLabel = computed((): string => stripRequiredLabel(props.label, isRequired.value))

const filterQuery = ref<string>('')
Expand All @@ -382,7 +388,6 @@ const uniqueFilterQuery = computed((): boolean => {

const selectWrapperId = useUniqueId() // unique id for the KPop target
const selectedItem = ref<SelectItem | null>(null)
const selectId = attrs.id ? String(attrs.id) : useUniqueId()
const selectItems = ref<SelectItem[]>([])
const inputFocused = ref<boolean>(false)

Expand Down Expand Up @@ -584,6 +589,20 @@ const onOpen = (toggle: () => void) => {
toggle()
}

const setLabelAttributes = () => {
/**
* Temporary fix for the issue where we can't use v-bind-once to pass id to a custom element (KInput)
* TODO: remove this once useId is released in Vue 3.5
*/
if (!attrs.id) {
const inputElementId = inputElement.value?.$el?.querySelector('input')?.id

if (inputElementId) {
labelElement.value?.$el?.setAttribute('for', inputElementId)
}
}
}

watch(value, (newVal, oldVal) => {
if (newVal !== oldVal) {
const item = selectItems.value?.filter((item: SelectItem) => item.value === newVal)
Expand Down Expand Up @@ -664,6 +683,12 @@ watch(selectedItem, (newVal, oldVal) => {
}
}, { deep: true })

watch(() => attrs.id, async () => {
inputKey.value++
await nextTick()
setLabelAttributes()
}, { immediate: true })

onMounted(() => {
if (selectWrapperElement.value) {
resizeObserver.value = ResizeObserverHelper.create(() => {
Expand All @@ -672,6 +697,8 @@ onMounted(() => {

resizeObserver.value.observe(selectWrapperElement.value as HTMLDivElement)
}

setLabelAttributes()
})

onUnmounted(() => {
Expand Down