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

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

Open
1 task done
ehni opened this issue Oct 2, 2024 · 7 comments
Assignees
Labels
bug This issue is a bug in the code

Comments

@ehni
Copy link

ehni commented Oct 2, 2024

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

  • I’m not disclosing any internal or sensitive information.
@ehni ehni added the bug This issue is a bug in the code label Oct 2, 2024
@ehni ehni changed the title [ui5-select | vue]: [ui5-select | vue]: Select does not recognise :selected property and instead wrongly shows last item as selected Oct 2, 2024
@pskelin
Copy link
Contributor

pskelin commented Oct 2, 2024

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
Copy link
Contributor

pskelin commented Oct 2, 2024

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
Copy link
Author

ehni commented Oct 2, 2024

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
Copy link
Author

ehni commented Oct 2, 2024

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
Copy link
Contributor

pskelin commented Oct 2, 2024

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
Copy link
Contributor

pskelin commented Oct 2, 2024

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">
  1. 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
Copy link
Author

ehni commented Oct 4, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug in the code
Projects
None yet
Development

No branches or pull requests

2 participants