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

feat(ui5-filter-item, ui5-sort-item,..): add filterItems to ui5-confirm event details #9838

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

hinzzx
Copy link
Contributor

@hinzzx hinzzx commented Sep 10, 2024

Previously, when confirming the selection in the View Settings Dialog, the visible UI text of the selected item was returned. However, this text doesn't always match the value required by the backend service for example, and it can also change due to translations or other factors, creating inconsistencies.

To address this, we are now providing the slotted ui5-filter-item's in the ui5-confirm event details.

This way the app developers could be able to selectively filter the selected items by property of their liking (for example by adding data-key (or similar) attribute/property to the ui5-filter-item-option's or ui5-filter-item's);

Sample usage:

<ui5-button id="btnOpenDialogWithDataSet">Show ViewSettingsDialog</ui5-button>
<ui5-view-settings-dialog id="vsdWithDatasetDataSet">
	<ui5-sort-item slot="sortItems" text="Name" data-key="sortName"></ui5-sort-item>
	<ui5-sort-item slot="sortItems" text="Position" data-key="sortPosition"></ui5-sort-item>
	<ui5-sort-item slot="sortItems" text="Company" data-key="sortCompany"></ui5-sort-item>

	<ui5-filter-item slot="filterItems" text="Category" data-key="filterCategory">
		<ui5-filter-item-option slot="values" text="Electronics" data-key="filterOptionElectronics" selected></ui5-filter-item-option>
		<ui5-filter-item-option slot="values" text="Clothing" data-key="filterOptionClothing"></ui5-filter-item-option>
		</ui5-filter-item>

		<ui5-filter-item slot="filterItems" text="Color" data-key="filterColor">
			<ui5-filter-item-option slot="values" text="Red" data-key="filterOptionRed"></ui5-filter-item-option>
			<ui5-filter-item-option slot="values" text="Blue" data-key="filterOptionBlue"></ui5-filter-item-option>
		</ui5-filter-item>

	</ui5-view-settings-dialog>

<script>
	btnOpenDialogWithDataSet.addEventListener("click", function () {
		vsdWithDatasetDataSet.open = true;
	});

	vsdWithDatasetDataSet.addEventListener("ui5-confirm", function(e) {
		const filterItems = e.detail.filterItems || [];

		// Get all selected filter item options
		const selectedFilterItems = filterItems.map(filterItem => {
			return {
				text: filterItem.text,
				values: filterItem.values.filter(filterItemOption => filterItemOption.selected)
			};
		}).filter(filterItem => filterItem.values.length > 0);

		// Get all NOT selected filter item options
		const notSelectedFilterItems = filterItems.map(filterItem => {
			return {
				text: filterItem.text,
				values: filterItem.values.filter(filterItemOption => !filterItemOption.selected)
			};
		}).filter(filterItem => filterItem.values.length > 0);

		// Log all the Selected items.
		if (selectedFilterItems.length > 0) {
			const firstSelectedFilterItem = selectedFilterItems[0];
			const firstSelectedFilterItemKey = firstSelectedFilterItem.values[0].dataset.key;
			const firstSelectedFilterItemText = firstSelectedFilterItem.values[0].text;
				
			console.log(`First selected filter item key: ${firstSelectedFilterItemKey}`);
			console.log(`First selected filter item text: ${firstSelectedFilterItemText}`);
		} else {
			console.log("No filters selected");
		}

		// Log all the NOT Selected items.
		if (notSelectedFilterItems.length > 0) {
			const firstNotSelectedFilterItem = notSelectedFilterItems[0];
			const firstNotSelectedFilterItemKey = firstNotSelectedFilterItem.values[0].dataset.key;
			const firstNotSelectedFilterItemText = firstNotSelectedFilterItem.values[0].text;
				
			console.log(`First not selected filter item key: ${firstNotSelectedFilterItemKey}`);
			console.log(`First not selected filter item text: ${firstNotSelectedFilterItemText}`);
		} else {
			console.log("No filters remain unselected");
		}
		alert("OK button clicked, returned info is: " + JSON.stringify(e.detail));
</script>

Fixes: #7579

Copy link
Contributor

@NHristov-sap NHristov-sap left a comment

Choose a reason for hiding this comment

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

Hi @hinzzx ,

Adding itemKey property to items is a part of the whole task, other part is to return whole items as confirm event detail so developer can distinguish selected filter items/options by their key. Now the event parameter still contains only text values.

Also, in the provided sample there's no idea what happens when the VSD is closed.

Best Regards,
Nikolay Hristov

Copy link
Contributor

@NHristov-sap NHristov-sap left a comment

Choose a reason for hiding this comment

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

In my opinion, it seems redundant to have first selected filters with all their options inside (no matter selected or not, but there is property for this), and second - the same list of just selected options. If there is no (easy) way to list just selected filter item (without its options), the second (selected options list) is maybe pointless.

@hinzzx
Copy link
Contributor Author

hinzzx commented Sep 16, 2024

In my opinion, it seems redundant to have first selected filters with all their options inside (no matter selected or not, but there is property for this), and second - the same list of just selected options. If there is no (easy) way to list just selected filter item (without its options), the second (selected options list) is maybe pointless.

Edit:
After a discussion we decided to remove the second list.

@hinzzx hinzzx marked this pull request as ready for review September 19, 2024 13:28
Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

The only thing that needs to be done is to add one line that returns the item in addition to the itemText

{
 itemText: item.text,
 item,
 }

Тhis is enough for the application. They can set their own properties on the items to differentiate them, as long as we return the item instead of its text, so they have the reference.

In UI5 Web Components we do not create properties that are not used for the rendering and only serve to help the application, because this is HTML, and any application can set any properties on our HTML elements.

The whole application logic that you're providing with this change can be achieved by just adding this one line and then the application setting itemKey the same way as in your example, or any other property with any name they like. It doesn't need to be a component/metadata property, because it has nothing to do with the component itself. We do such things in OpenUI5, because there apps don't have access to HTML directly and it's more inconvenient to set custom stuff (f.e. they can only use LayoutData)

selected: false,
filterOptions: item.values.map(optionValue => {
return {
text: optionValue.text || "",
itemKey: optionValue.itemKey || "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to: item: optionValue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I can even remove that line, as we can just return the slotted filterItems in the get eventsParams(). This way the app developers could execute their own logic for filtering the selected items based on their HTML Property they have provided (f.e data-key);

@@ -624,6 +632,7 @@ class ViewSettingsDialog extends UI5Element {
sortBy,
sortByItem,
filters: this.selectedFilters,
filterItems: this.selectedFilterItems,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this new field really necessary? Can't the app do this if they need such information? Or is this code required by the control somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The app cannot do this themself because until now we only return the text of the provided filters in the details, therefore we would have to provide the ui5-filter-item, and its ui5-filter-item-option's in the event details.
This way providing for example data-key="filterText1" to the ui5-filter-item or ui5-filter-item-option would make them able to be accessed by $0.dataset.key HTML Property.

@hinzzx hinzzx changed the title feat(ui5-filter-item, ui5-sort-item,..): add item-key property feat(ui5-filter-item, ui5-sort-item,..): add filterItems to ui5-confirm event details Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ViewSettingsDialog | ui5-webcomponents] onConfirmEvent uses filter item text as key:
4 participants