TW-Elements icon indicating copy to clipboard operation
TW-Elements copied to clipboard

Multiple select: calling setValue closing the opened dropdown

Open Nurai1 opened this issue 1 year ago • 13 comments

At select tw element with multiple flag - calling setValue closing the opened dropdown. Moreover, the '_isOpen' flag in state stays true after closing dropdown.

Advice: Check this code this.optionsWrapper.removeChild(optionsWrapperContent); this.optionsWrapper.appendChild(optionsListTemplate);

Steps to reproduce the behavior: Create multiple select and try calling setValue and see the bug

Nurai1 avatar Oct 20 '23 12:10 Nurai1

Can you prepare a snippet with an example on how to recreate the issue? Or provide some code here?

Link to where you can create a code snippet: https://tw-elements.com/snippets/

I tried to recreate but for me the setValue method wasn't closing the dropdown. You can check my snippet here: https://tw-elements.com/snippets/tailwind/b-cylwik/5730615#html-tab-view

juujisai avatar Oct 23 '23 06:10 juujisai

I'm sorry, I work with react environment and this is huge to prepare whole ready to use workspace, because I need to prepare Vite, tailwind, react etc.

This is the code with the error, I need setValue in useEffect for reactivity. So this error comes from outsideClick, first setValue change the dropdown wrapper, then dosument click handler check the dropdown wrapper, didn't find the right one because setValue changed it already and close the dropdown.

I can understand if you didn't take this, this seems confusing enough @juujisai Thanks for reaching out though

Nurai1 avatar Oct 23 '23 08:10 Nurai1

import { find } from "lodash-es";
import { PrivoMultipleSelectOption } from "privo-shared/src/tailwind-components/forms/PrivoMultipleSelect";
import React, { ChangeEventHandler, useRef } from "react";
import { Input, Select, initTE } from "tw-elements";
const options = [
    {
        name: "Mexico. The land of tequila and quesadillas",
        value: "MexicoVal",
    },
    {
        name: "Brazil",
        value: "BrazilVal",
    },
    {
        name: "Chili",
        value: "ChiliVal",
    },
];
const App: React.FCWithChildren = () => {
    const select = useRef<typeof Select>();
    const [outerValue, setOuterValue] = React.useState<any[]>([]);
    const handleReferenceChange = (el: HTMLSelectElement | null) => {
        if (el && !select.current) {
            select.current = new Select(el);
        }
    };
    const handleChange: ChangeEventHandler<HTMLSelectElement> = () => {
        const values = select.current.value.filter(Boolean) as string[];
        const innerOptions = values?.map((v) =>
            find(options, (o) => String(o.value) === String(v))
        ) as PrivoMultipleSelectOption[];
        setOuterValue(innerOptions);
    };
    React.useEffect(() => {
        if (select.current) {
            // the error comes from here, dropdown will be closed after calling setValue
            select.current.setValue(outerValue.map((s) => s.value));
        }
    }, [outerValue]);
    React.useEffect(() => {
        initTE({ Select, Input });
    }, []);
    return (
        <div>
            <div></div>
            <select
                ref={handleReferenceChange}
                onChange={handleChange}
                id="select-one"
                data-te-select-init
                data-te-select-all="false"
                data-te-multiple="true"
                multiple
            >
                <option value="" hidden></option>
                {options.map((o) => (
                    <option key={o.name} value={o.value}>
                        {o.name}
                    </option>
                ))}
            </select>
        </div>
    );
};
export default App;

Nurai1 avatar Oct 23 '23 08:10 Nurai1

The problem is with this line

select.current.setValue(outerValue.map((s) => s.value));

Because the wrapper doesn't have the setValue method that would change the Select component. You have to get the instance and then call the setValue there:

  React.useEffect(() => {
    if (select.current) {
      // the error comes from here, dropdown will be closed after calling setValue
      Select.getInstance(select.current)?.setValue(
        outerValue.map((s) => s.value)
      );
    }
  }, [outerValue]);

After the change, the select options do not close the Select dropdown.

juujisai avatar Oct 24 '23 05:10 juujisai

@juujisai The problem is not there, because select.current it is already an instance. Look at this row:

const handleReferenceChange = (el: HTMLSelectElement | null) => {
        if (el && !select.current) {
            select.current = new Select(el);
        }
    };

Nurai1 avatar Nov 09 '23 12:11 Nurai1

I see, yes you are right. I do not seem to understand the need for select.current.setValue(outerValue.map((s) => s.value)); though. When I delayed the execution of this code with setTimeout the dropdown does not close anymore but it resets the value to an empty array.

      console.log(outerValue); // returns an empty array
      setTimeout(() => {
        console.log(outerValue); // returns an array with value "false"
        select.current.setValue(outerValue.map((s) => s.value));
      }, 100);

Is there a need to use a setValue method?

juujisai avatar Nov 09 '23 15:11 juujisai

I have got the same issue.

xavier-ottolini avatar Nov 20 '23 15:11 xavier-ottolini

@xavier-ottolini Hi! When are you calling the setValue method? What stack are you using? Can you provide some example?

juujisai avatar Nov 21 '23 06:11 juujisai

I use VueJS 3

Main.vue

<script setup lang="ts">
import { Ref } from 'vue'
import MySelect from './MySelect.vue'
const selectOptions = [
  { value: 'Value 1', label: 'Option 1' },
  { value: 'Value 2', label: 'Option 2' },
  { value: 'Value 3', label: 'Option 3' },
  { value: 'Value 4', label: 'Option 4' },
  { value: 'Value 5', label: 'Option 5' },
  { value: 'Value 6', label: 'Option 6' },
]
const selectValues: Ref<string[]> = ref([])
</script>
<template>
            <my-select v-model="selectValues" placeholder="Select multiple option" multiple>
              <my-option v-for="option in selectOptions" :label="option.label" :value="option.value" />
            </my-select>
</template>

MyOption.vue

<script setup lang="ts">
const props = defineProps<{
  value: any
  label?: string
  attributes?: Object
}>()

</script>
<template>
  <option
    v-bind="attributes"
    :value="value"
  >
    <slot>
      {{ label }}
    </slot>
  </option>
</template>

The MySelect.vue components uses te.Select.

<script setup lang="ts">
import { onMounted, ref, watch, useSlots } from 'vue'
import * as te from 'tw-elements'
import { v4 as uuidv4 } from 'uuid'
import { Select, initTE } from "tw-elements"

const props = defineProps<{
  name?: string
  modelValue?: any
  multiple?: boolean
  placeholder?: string
  required?: boolean
  attributes?: Object
}>()

const emit = defineEmits(['update:modelValue'])

const selectRef = ref<HTMLSelectElement>()
const selectId = `select-${uuidv4()}`

const slots = useSlots()

function onChange(event: Event) {
  if(props.multiple) {
    const values: Array<any> = Array.from((event.target as HTMLSelectElement).selectedOptions).map(option => option.value)
    emit('update:modelValue', values)
  }
  else {
    const value = (event.target as HTMLSelectElement).value
    emit('update:modelValue', value)
  }
}

const selectInstance = ref()

const setValue = (value: any) => {
  selectInstance.value.setValue(value)
}

watch(() => props.modelValue, setValue)

onMounted(() => {
  initTE({ Select })
  selectInstance.value = te.Select.getOrCreateInstance(selectRef.value)
  setValue(props.modelValue)
})

const selectClass = 'block w-full rounded border-gray-300 shadow-sm focus:border-primary-300 focus:ring focus:ring-primary-200 focus:ring-opacity-50 disabled:bg-gray-100'
</script>

<template>
  <select
    ref="selectRef"
    :id="selectId"
    data-te-select-init
    :data-te-select-placeholder="placeholder"
    :data-te-class-select-input="selectClass"
    :multiple="multiple"
    :name="name"
    :required="required"
    @change="onChange"
  >
    <slot></slot>
  </select>
</template>
</script>```

xavier-ottolini avatar Nov 21 '23 08:11 xavier-ottolini

@xavier-ottolini

I'm adding this to our project since I can't find a solution for that at this very moment. There seem to be some kind of weird interaction when doing two ways binding in frameworks - in vanilla JS it doesn't close the dropdown :/

We'll try to find a solution for that

juujisai avatar Nov 21 '23 13:11 juujisai

Thank you !

xavier-ottolini avatar Nov 21 '23 13:11 xavier-ottolini

I've been testing this issue again. Sorry it took some time, I'm not sure whether it still will be useful for you but maybe someone other may make use of that.

From what I can see the issue occurs when we are trying to send the data with use of setValue too soon after selects change event emits. Those updates looks like they overlap and create this weird behavior where instance looks different than before and our click outside method treats the click like a click outside and closes the dropdown. I'm not sure whether I explained this clear enough because I'm still trying to find out why exactly there are different instance(?) there.

I'm going to still look at this later but I cannot promise anything.

There are few things we can do though:

  1. Add delay, like it was said before - setTimeouts - this can make some weird animations appear (delayed as we can expect)
  2. Make sure to call the setValue method when the selected data actually changed (for example outside the component or after some custom code inside your app)

@Nurai1 I'm not sure if inside the example you have provided the setValue method makes sense. The component emits event when the value inside changed and adding it back almost instantly is not necessary. If the outerValue will be changed other way than inside the change event handler, then you can add a if statement that checks whether the value changed

@xavier-ottolini Inside you vue app the check whether the value changed works properly for me and I think it would be the correct way to handle this issue. Calling of the setValue method is not needed after we check the value inside the change event, send it to the parent via emit and then update it again from the watcher. Checkout this snippet

const selectValue = ref<any>(null);

function onChange(event: Event) {
  if (props.multiple) {
    selectValue.value = Array.from(
      (event.target as HTMLSelectElement).selectedOptions
    ).map((option) => option.value);
    emit("update:modelValue", selectValue.value);
  } else {
    selectValue.value = (event.target as HTMLSelectElement).value;
    emit("update:modelValue", selectValue.value);
  }
}

const selectInstance = ref();

const setValue = (value: any) => {
  if (value === selectValue.value) {
    return;
  }
  selectInstance.value.setValue(value);
};

Let me know if I missed something here and it still causes issues.

juujisai avatar Dec 15 '23 12:12 juujisai

const setValue = (value: any) => {
  if (value === selectValue.value) {
    return;
  }
  selectInstance.value.setValue(value);
};

Yes, I confirm that it is necessary to check that the value has not changed before to call selectInstance.value.setValue(value);. This workaround works.

xavier-ottolini avatar Dec 15 '23 14:12 xavier-ottolini