SAP / ui5-webcomponents

UI5 Web Components - the enterprise-flavored sugar on top of native APIs! Build SAP Fiori user interfaces with the technology of your choice.
https://sap.github.io/ui5-webcomponents/
Apache License 2.0
1.58k stars 270 forks source link

[ui5-select | vue]: Select does not recognise `:selected` property and instead wrongly shows last item as selected #9970

Closed ehni closed 1 month ago

ehni commented 2 months ago

Bug Description

Select shows not the selected option but instead the last one.

Affected Component

ui5-select

Expected Behaviour

Should show selected option as selected (in the button)

Isolated Example

https://github.com/ehni/vue-ui5-webcomponents-playground

Here you can see a screenshot on how it looks when I open the page or refresh:

image

And here is the corresponding code used, see also https://github.com/ehni/vue-ui5-webcomponents-playground/blob/main/src/App.vue:

<template>
    <div>
        <h1>Select problem</h1>
        <p>Problem: Select does not show selected item. "All" (first item) should be selected after page load with the
            ":selected" property, instead the third/last item ("Completed") is selected.</p>
        <ui5-select @change="updateStateFilter($event)">
            <ui5-option v-for="stateFilter in stateFilters" :key="stateFilter.id" :data-value="stateFilter.id"
                :selected="currentStateFilter.toLowerCase() === stateFilter.id.toLowerCase()">
                {{ stateFilter.displayName }} (selected: {{ currentStateFilter.toLowerCase() ===
                    stateFilter.id.toLowerCase() }})
            </ui5-option>
        </ui5-select>
        <p>Current state filter: {{ currentStateFilter }}</p>
        <p>Possible filters:</p>
        <ul>
            <li v-for="filter in stateFilters" :key="filter.id">
                id: {{ filter.id }} - displayName: {{ filter.displayName }}
            </li>
        </ul>
    </div>
</template>

<script>
import '@ui5/webcomponents/dist/Select.js';
import '@ui5/webcomponents/dist/Option.js';

export default {
    components: {
    },
    data() {
        return {
            currentStateFilter: 'all',
            stateFilters: [
                { id: 'all', displayName: 'All' },
                { id: 'active', displayName: 'Active' },
                { id: 'completed', displayName: 'Completed' }
            ],
        }
    },
    methods: {
        updateStateFilter(event) {
            const targetValue = event.detail.selectedOption.dataset.value
            console.log('updating state: ', targetValue);
            this.currentStateFilter =
                this.stateFilters.find(filter => filter.displayName.toLowerCase() === targetValue.toLowerCase())?.id || 'all';
        },
        onReturnPressed() {
            console.log('return pressed');
            alert('return pressed');
        }

    }
};
</script>

<style></style>

Steps to Reproduce

  1. Checkout repo
  2. Run npm install & npm run dev
  3. Open page.
  4. Should show "All" as selected but instead shows "Completed"

Log Output, Stack Trace or Screenshots

No response

Priority

Medium

UI5 Web Components Version

Latest? I just cloned your example repository.

Browser

Edge

Operating System

MacOS

Additional Context

No response

Organization

SAP

Declaration

pskelin commented 2 months ago

Thanks @ehni for taking the time to make a good reproduction.

I could test this really fast and can confirm that it is fixed with #9857 which will be part of the 2.3 release (a couple of days at most).

As a temporary workaround, you can use the property assignement syntax

.selected="currentStateFilter.toLowerCase() === stateFilter.id.toLowerCase()"

or the longhand syntax which is the same

:selected.prop="currentStateFilter.toLowerCase() === stateFilter.id.toLowerCase()"

After the fix is released, this will no longer be necessary and :selected will work as expected.

pskelin commented 2 months ago

btw, if you bind a value attribute to the options, you can also bind the value property to the select and there is no need to calculate the selected of each option.

- <ui5-select @change="updateStateFilter($event)">
+ <ui5-select @change="updateStateFilter($event)" .value="currentStateFilter">
- <ui5-option v-for="stateFilter in stateFilters" :key="stateFilter.id" :data-value="stateFilter.id"
-     :selected.....>
+ <ui5-option v-for="stateFilter in stateFilters" :key="stateFilter.id" :value="stateFilter.id">

Additionally, I have created a feature request that will further simplify vuejs usage. When it is completed, it will be possbile to use the select like the native one with v-model directly

<ui5-select v-model="select">
  <ui5-option value="a">A</ui5-option>
  <ui5-option value="b">B</ui5-option>
  <ui5-option value="c">C</ui5-option>
</ui5-select>

You can follow the progress here: #9971

ehni commented 2 months ago

Thanks for the quick & helpful response.

btw, if you bind a value attribute to the options, you can also bind the value property to the select and there is no need to calculate the selected of each option.

While this makes the UI5 Select behave an look like I expect, I still need to manually update the currentStateFilter value or? The component does not Update the value.

ehni commented 2 months ago

Which directly brings me to me next question: is there any way to just provide / get the iterated element in the change event without the need to traverse all the event chain which feels kind of hacky:

updateStateFilter(event) {
    const targetValue = event.detail.selectedOption.dataset.value
    console.log('updating state: ', targetValue);
    this.currentStateFilter =
        this.stateFilters.find(filter => filter.displayName.toLowerCase() === targetValue.toLowerCase())?.id || 'all';
},

Instead what I'd expect and being used to from other Frameworks is either being able to define the click function directly on sub element itself:

<ui5-select :value="currentStateFilter">
    <ui5-option v-for="stateFilter in stateFilters" :key="stateFilter.id" :value="stateFilter.id"
        :select="updateStateFilter(stateFilter)">
        {{ stateFilter.displayName }} 
    </ui5-option>
</ui5-select>

Or to provide a way to retrieve the iterated data object without having to manually define some keys and then find and match the items by myself.

Btw: the same applies for other UI5 Components like list and table. The provided CustomEvents are quite frustrating to handle and seem to be not intuitive (maybe I'm just missing something?).

pskelin commented 2 months ago

Thanks for the quick & helpful response.

btw, if you bind a value attribute to the options, you can also bind the value property to the select and there is no need to calculate the selected of each option.

While this makes the UI5 Select behave an look like I expect, I still need to manually update the currentStateFilter value or? The component does not Update the value.

oh yes, the @change="updateStateFilter($event)" handler is still needed until we implement #9971

Here is the full code I used, which at least simplifies the :seleted calculation

<template>
    <div>
        <h1>Select problem</h1>
        <p>Problem: Select does not show selected item. "All" (first item) should be selected after page load with the
            ":selected" property, instead the third/last item ("Completed") is selected.</p>
        <ui5-select @change="updateStateFilter($event)" .value="currentStateFilter">
            <ui5-option v-for="stateFilter in stateFilters" :key="stateFilter.id" :value="stateFilter.id">
                {{ stateFilter.displayName }} (selected: {{ currentStateFilter.toLowerCase() ===
                    stateFilter.id.toLowerCase() }})
            </ui5-option>
        </ui5-select>
        <p>Current state filter: {{ currentStateFilter }}</p>
        <p>Possible filters:</p>
        <ul>
            <li v-for="filter in stateFilters" :key="filter.id">
                id: {{ filter.id }} - displayName: {{ filter.displayName }}
            </li>
        </ul>
    </div>
</template>

<script>
import '@ui5/webcomponents/dist/Select.js';
import '@ui5/webcomponents/dist/Option.js';

export default {
    components: {
    },
    data() {
        return {
            currentStateFilter: 'active',
            stateFilters: [
                { id: 'all', displayName: 'All' },
                { id: 'active', displayName: 'Active' },
                { id: 'completed', displayName: 'Completed' }
            ],
        }
    },
    methods: {
        updateStateFilter(event) {
            const targetValue = event.detail.selectedOption.value
            console.log('updating state: ', targetValue);
            this.currentStateFilter =
                this.stateFilters.find(filter => filter.displayName.toLowerCase() === targetValue.toLowerCase())?.id || 'all';
        },
        onReturnPressed() {
            console.log('return pressed');
            alert('return pressed');
        }

    }
};
</script>

<style></style>
pskelin commented 2 months ago

Which directly brings me to me next question: is there any way to just provide / get the iterated element in the change event without the need to traverse all the event chain which feels kind of hacky:

updateStateFilter(event) {
  const targetValue = event.detail.selectedOption.dataset.value
  console.log('updating state: ', targetValue);
  this.currentStateFilter =
      this.stateFilters.find(filter => filter.displayName.toLowerCase() === targetValue.toLowerCase())?.id || 'all';
},

Instead what I'd expect and being used to from other Frameworks is either being able to define the click function directly on sub element itself:

<ui5-select :value="currentStateFilter">
  <ui5-option v-for="stateFilter in stateFilters" :key="stateFilter.id" :value="stateFilter.id"
      :select="updateStateFilter(stateFilter)">
      {{ stateFilter.displayName }} 
  </ui5-option>
</ui5-select>

Or to provide a way to retrieve the iterated data object without having to manually define some keys and then find and match the items by myself.

Btw: the same applies for other UI5 Components like list and table. The provided CustomEvents are quite frustrating to handle and seem to be not intuitive (maybe I'm just missing something?).

sorry, I don't get this question.

From what I understand, you want to have a state that is in sync with the select. If you update the select, the state is updated and the other way around

  1. updating select from state you just write

    <ui5-select .value="state">
  2. updating the state from the select

    <ui5-select @change="e => state = e.selectedOption.value">

    there is no need to iterate the options. Perhaps you can explain in more detail?

ehni commented 1 month ago

Nevermind I wanted to create your an example with the Table element and I think I already found a solution by myself:

<template>
    <div style="max-width: 300px">
        <h1>Table</h1>
        <ui5-table id="table" overflow-mode="Popin">
            <ui5-table-header-row slot="headerRow">
                <ui5-table-header-cell id="currentFilterCol" width="20px"></ui5-table-header-cell>
                <ui5-table-header-cell id="filterIdCol"><span>ID</span></ui5-table-header-cell>
                <ui5-table-header-cell id="filterDisplayNameCol">Name</ui5-table-header-cell>
            </ui5-table-header-row>
            <ui5-table-row v-for="stateFilter in stateFilters" :key="stateFilter.id" interactive
                @click="tableRowClicked(stateFilter)">
                <ui5-table-cell><ui5-label>{{ currentStateFilter === stateFilter.id ? '☑️' : '' }}
                    </ui5-label></ui5-table-cell>
                <ui5-table-cell><ui5-label>{{ stateFilter.id
                        }}</ui5-label></ui5-table-cell>
                <ui5-table-cell><ui5-label>{{ stateFilter.displayName }}</ui5-label></ui5-table-cell>
            </ui5-table-row>
        </ui5-table>
    </div>
</template>

<style></style>

<script setup>
import "@ui5/webcomponents/dist/Table.js";
import "@ui5/webcomponents/dist/TableHeaderRow.js";
import "@ui5/webcomponents/dist/TableHeaderCell.js";
import "@ui5/webcomponents/dist/Label.js";
import { ref } from "vue";

const currentStateFilter = ref('all');
const stateFilters = ref(
    [
        { id: "all", displayName: "All" },
        { id: "active", displayName: "Active" },
        { id: "completed", displayName: "Completed" },
    ]
);

function tableRowClicked(filter) {
    console.log('clicked: ', filter);
    currentStateFilter.value = filter.id;
};
</script>

Instead of using the row-click event I simply used the native click element on the row itself.