Several issues with BFormTags

dwgray commented 1 month ago

Describe the bug

I found several bugs/parity issues with this component that were more than I could easily fix with my routine updates.

There is something not quite working with custom components, possibly with how the scoped slots are being passed through - although there is some possibility this is just a doc bug. The following doc examples don't work:
1. https://bootstrap-vue-next.github.io/bootstrap-vue-next/docs/components/form-tags.html#using-custom-form-components
2. https://bootstrap-vue-next.github.io/bootstrap-vue-next/docs/components/form-tags.html#advanced-custom-rendering-usage
feedback-aria-live does not appear to be implemented
ignore-input-focus-selector does not appear to be implemented

I'm not generally a fan of aggregate bugs like this, but I'm trying to minimize overhead while still getting issues in front of people who may be able to fix them. I'm happy to adjust how I track this based on feedback.

Reproduction

https://bootstrap-vue-next.github.io/bootstrap-vue-next/docs/components/form-tags.html#using-custom-form-components

Used Package Manager

pnpm

Validations

[X] Have tested with the latest version. This is still alpha version and sometime things change rapidly.
[X] Follow our Code of Conduct
[X] Read the Contributing Guide.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A, please open a GitHub Discussion instead.
[X] The provided reproduction is a minimal reproducible of the bug.

matsaal commented 1 month ago

I think I've resolved the first issue and part of the second; the other issue with the second example is that the input event is firing on each keystroke rather than on the tag being submitted as in BootstrapVue.

dwgray commented 1 month ago

I think I've resolved the first issue and part of the second; the other issue with the second example is that the input event is firing on each keystroke rather than on the tag being submitted as in BootstrapVue.

Thanks @matsaal. I agree that you've fixed 1.i. and helped with 1.ii. (I've changed the bullet lists to ordered for easier reference).

I'll update the parity spreadsheet shortly (unless you're planning on doing more of this kind of fix, in which case we can look at getting you access to the parity spreadsheet directly.

In looking at 1.ii - it seems like the problem is that the inputAttrs and inputHandler from BFormTags used to line up with events and properties of BFormSelect in a way that just worked. I played around a bit with things to try to get them to work, but am running into a cognitive disconnect with how vue models work.

I've got a slightly improved repro here:

https://stackblitz.com/edit/github-renbwj?file=src%2Fcomponents%2FComp.vue

<template>
  <div>
    <BFormGroup label="Tagged input using select" label-for="tags-component-select">
      <BFormTags
        id="tags-component-select"
        v-model="customPredefinedTags"
        size="lg"
        class="mb-2"
        no-outer-focus
      >
        <template #default="{tags, disabled, addTag, removeTag}">
          <ul v-if="tags.length > 0" class="list-inline d-inline-block mb-2">
            <li v-for="tag in tags" :key="tag" class="list-inline-item">
              <BFormTag :title="tag" :disabled="disabled" variant="info" @remove="removeTag">{{
                tag
              }}</BFormTag>
            </li>
          </ul>
          <BFormSelect
            v-model="customPredfinedSelect"
            :disabled="disabled || availableOptions.length === 0"
            :options="availableOptions"
            @update:model-value="addTag"
          >
            <template #first>
              <!-- This is required to prevent bugs with Safari -->
              <option disabled value="">Choose a tag...</option>
            </template>
          </BFormSelect>
        </template>
      </BFormTags>
    </BFormGroup>
    {{ customPredfinedSelect }}
  </div>
</template>

<script setup lang="ts">
import {computed, ref} from 'vue'

const customPredefinedTags = ref<string[]>([])
const options = ref<string[]>([
  'Apple',
  'Orange',
  'Banana',
  'Lime',
  'Peach',
  'Chocolate',
  'Strawberry',
])
const availableOptions = computed(() =>
  options.value.filter((opt) => customPredefinedTags.value.indexOf(opt) === -1)
)

const customPredfinedSelect = computed({
  get: () => {
    // eslint-disable-next-line no-console
    console.log('Calling get_customPredfinedSelect')
    return ''
  },
  set: (newValue: string) => {
    // eslint-disable-next-line no-console
    console.log('Calling get_customPredfinedSelect', newValue)
  },
})
</script>

Replace the inputHandler binding with a direct binding to addTag for the @update:model-value event and removing the finding to inputAttrs gets just about everything working (although we lose some useful bindings with inputAttrs.

The thing that is still completely broken is that when you select something in the BFormSelect, it ends up setting up the BFormTags collection correctly, but it also tried to set the value of the BFormSelect and ends up setting it to something else in the list, since the value is removed from the BFormSelect when it's added to the BFormTags. I tried a number of things to get that to stop, but I must be mis-understanding something about how model binding works because I can't block the value from being set.

In my example, I created a settable computed for the BFormSelect model value with some logging while the setter is getting called when expected, the getter is never called - yet the value is changing to what was selected. I also (earlier on) tried just passing an empty string as the model-value and that appeared to be ignored.

I'm going to pause on this for now, but if @matsaal or @VividLemon has any ideas (which can include - "you missed this really obvious thing"), please let me know.

matsaal commented 1 month ago

@dwgray I think I've found the reason why the form select example in the docs doesn't work, so once I've confirmed that I've got it right and tidied it up a bit I'll submit it for review.

dwgray commented 1 month ago

Thanks @matsaal. I look forward to seeing the solution.

github-actions[bot] commented 1 week ago

This issue is stale because it has been open for 30 days with no activity. It will not be auto-closed

bootstrap-vue-next / bootstrap-vue-next