feat(ui5-view-settings-dialog): introduce filter section #3616

fifoosid · 2021-08-10T06:27:21Z

Fixes #3451
Fixes #3687

BREAKING CHANGE:
ui5-view-settings-dialog now uses ui5-sort-item instead of ui5-li.

packages/fiori/src/FilterItemOption.js

packages/fiori/src/ViewSettingsDialog.js

tsanislavgatev

Yet to review the VSD.js

tsanislavgatev · 2021-08-24T09:04:45Z

packages/fiori/src/i18n/messagebundle.properties

@@ -173,7 +173,7 @@ WIZARD_NAV_ARIA_ROLE_DESCRIPTION = Wizard
 WIZARD_NAV_STEP_DEFAULT_HEADING=Step

 #XTIT: View Settings Dialog dialog title for Sort


Why do we change the title?

Because previously it was with wrong text

tsanislavgatev · 2021-08-24T09:05:44Z

packages/fiori/test/pages/ViewSettingsDialog.html

 	</ui5-view-settings-dialog>

 	<script>
 		btnOpenDialog.addEventListener("click", function () {
 			vsd.show();
 		});
 		vsd.addEventListener("confirm", function(evt) {
-			alert("OK button clicked, returned info is: " + JSON.stringify(evt.detail));
+			// alert("OK button clicked, returned info is: " + JSON.stringify(evt.detail));


Which of these two should we use? We can remove the other one

unazko · 2021-08-24T10:09:16Z

get _dialogTitle method in ViewSettingsDialog.js file returns "Sort" in all cases, even if we are on Filtering page.

packages/fiori/src/ViewSettingsDialog.js

tsanislavgatev

I also see a lot of nested loops, can we think of a cheaper solution? ( If already checked, It's okay.)

tsanislavgatev · 2021-08-24T10:53:34Z

packages/fiori/src/ViewSettingsDialog.js

+				}
+			}
+
+			filter.additionalText = !selectedCount ? "" : selectedCount;


IMO: It's more intuitive to be selectedCount ? selectedCount : "". But it's okay in this way too.

It is like this because of the linter

unazko · 2021-08-24T10:42:36Z

packages/fiori/src/ViewSettingsDialog.js

-			  sortOrderSelected = this._sortOrder.getSelectedItems(),
-			  sortBySelected = this._sortBy.getSelectedItems();
+		return {
+			sortOrder: JSON.parse(JSON.stringify(this.initSortOrderItems)),


Why we go through this? Could we just use this.initSortOrderItems object the way it is?

No, we can't use it because it would be a shallow copy of the object and in this case we need a deep copy as we have nested objects & arrays

packages/fiori/src/ViewSettingsDialog.js

unazko · 2021-08-24T10:53:04Z

packages/fiori/src/ViewSettingsDialog.js

@@ -346,28 +595,39 @@ class ViewSettingsDialog extends UI5Element {
 	 * @param {Object} settings
 	 */
 	_restoreSettings(settings) {
-		const sortOrderSelected = settings.sortOrder && settings.sortOrder.innerText,
-			  sortBySelected = settings.sortBy && settings.sortBy.innerText;
+		this._currentSettings = JSON.parse(JSON.stringify(settings));


Why do we need these conversions?

Already explained

packages/fiori/src/i18n/messagebundle.properties

unazko · 2021-08-30T08:25:55Z

I have no more comments.

ilhan007 · 2021-09-10T09:29:11Z

packages/fiori/src/ViewSettingsDialog.js

+
+		/**
+		 * Defines the <code>filterItems</code> list.
+		 * @type {sap.ui.webcomponents.fiori.ListItem}


Here we say use @type {sap.ui.webcomponents.fiori.ListItem}, but it's not entirely right.
The type is different now - FilterItem for filterItems and SortItem for sortItems. Also, I recommend adding a jsdoc to let the users know they need to import the SortItem, FIlterItem, FilterItemOption, because they are not auto-imported. Could be a note in the slots description or in the overview, or both.

ilhan007 · 2021-09-10T09:31:59Z

Could you also fix the merge conflict

ilhan007 · 2021-09-10T11:06:08Z

I encountered the following state:

Open the dialog
Switch to filter view
Select a Filter to navigate to the Filter subview
Check one of the options
Pres ESC
Open the Dialog again

packages/fiori/src/ViewSettingsDialog.js

wip

1a66593

fifoosid added the BREAKING CHANGE label Aug 10, 2021

Merge branch 'master' into vsd-filter

30667c1

ilhan007 added the SAP SF label Aug 11, 2021

fifoosid added 4 commits August 13, 2021 14:14

wip

a6e76c3

wip

7d5122b

wip

6124fec

implement the spec

8cb0afd

fifoosid changed the title ~~[WIP] feat(ui5-view-settings-dialog): introduce filter section~~ feat(ui5-view-settings-dialog): introduce filter section Aug 18, 2021

fifoosid added the for review label Aug 18, 2021

remove unused code

5b91a58

ilhan007 reviewed Aug 18, 2021

View reviewed changes

packages/fiori/src/FilterItemOption.js Outdated Show resolved Hide resolved

ilhan007 requested review from a team and DMihaylova August 18, 2021 20:48

fifoosid added 4 commits August 19, 2021 11:21

Merge branch 'master' into vsd-filter

b44b682

fix comments

480081b

fix bundle

0d18893

update docs

18a1948

fifoosid closed this Aug 19, 2021

fifoosid reopened this Aug 19, 2021

ilhan007 reviewed Aug 20, 2021

View reviewed changes

packages/fiori/src/ViewSettingsDialog.js Show resolved Hide resolved

unazko self-requested a review August 23, 2021 13:08

tsanislavgatev reviewed Aug 24, 2021

View reviewed changes

unazko reviewed Aug 24, 2021

View reviewed changes

packages/fiori/src/ViewSettingsDialog.js Outdated Show resolved Hide resolved

tsanislavgatev reviewed Aug 24, 2021

View reviewed changes

unazko reviewed Aug 24, 2021

View reviewed changes

fifoosid added 2 commits August 30, 2021 09:58

merge master branch

9b4e9ec

fix comments

79aad2d

fix more comments

5863347

fifoosid mentioned this pull request Aug 30, 2021

fix(ui5-view-settings-dialog): error not thrown in the console #3701

Closed

fifoosid requested a review from vladitasev August 31, 2021 07:28

ilhan007 reviewed Sep 10, 2021

View reviewed changes

fix comments

1be7afc

fifoosid requested a review from ilhan007 September 10, 2021 11:03

ilhan007 reviewed Sep 10, 2021

View reviewed changes

packages/fiori/src/ViewSettingsDialog.js Outdated Show resolved Hide resolved

fix comments

fe71314

ilhan007 approved these changes Sep 10, 2021

View reviewed changes

fifoosid merged commit 52987c2 into master Sep 10, 2021

fifoosid deleted the vsd-filter branch September 10, 2021 14:02

fifoosid added a commit that referenced this pull request Sep 10, 2021

feat(ui5-view-settings-dialog): introduce filter section (#3616)

89e07b1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ui5-view-settings-dialog): introduce filter section #3616

feat(ui5-view-settings-dialog): introduce filter section #3616

fifoosid commented Aug 10, 2021 •

edited

Loading

tsanislavgatev left a comment

tsanislavgatev Aug 24, 2021

fifoosid Aug 30, 2021

tsanislavgatev Aug 24, 2021

fifoosid Aug 30, 2021

unazko commented Aug 24, 2021

tsanislavgatev left a comment

tsanislavgatev Aug 24, 2021

fifoosid Aug 30, 2021 •

edited

Loading

unazko Aug 24, 2021 •

edited

Loading

fifoosid Aug 30, 2021

unazko Aug 24, 2021

fifoosid Aug 30, 2021

unazko commented Aug 30, 2021

ilhan007 Sep 10, 2021

ilhan007 commented Sep 10, 2021

ilhan007 commented Sep 10, 2021 •

edited

Loading

		@@ -173,7 +173,7 @@ WIZARD_NAV_ARIA_ROLE_DESCRIPTION = Wizard
		WIZARD_NAV_STEP_DEFAULT_HEADING=Step

		#XTIT: View Settings Dialog dialog title for Sort

feat(ui5-view-settings-dialog): introduce filter section #3616

feat(ui5-view-settings-dialog): introduce filter section #3616

Conversation

fifoosid commented Aug 10, 2021 • edited Loading

tsanislavgatev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unazko commented Aug 24, 2021

tsanislavgatev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fifoosid Aug 30, 2021 • edited Loading

Choose a reason for hiding this comment

unazko Aug 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unazko commented Aug 30, 2021

Choose a reason for hiding this comment

ilhan007 commented Sep 10, 2021

ilhan007 commented Sep 10, 2021 • edited Loading

fifoosid commented Aug 10, 2021 •

edited

Loading

fifoosid Aug 30, 2021 •

edited

Loading

unazko Aug 24, 2021 •

edited

Loading

ilhan007 commented Sep 10, 2021 •

edited

Loading