Editor: Add a search filters summary component #1037

tkohr · 2024-11-13T11:58:16Z

Description

This PR introduces components to display currently selected search filter values in a zone below the search filters.

To-dos:

format date
format user
finish UI
tests

Screenshots

Quality Assurance Checklist

Commit history is devoid of any merge commits and readable to facilitate reviews
If new logic ⚙️ is introduced: unit tests were added
If new user stories 🤏 are introduced: E2E tests were added
If new UI components 🕹️ are introduced: corresponding stories in Storybook were created
If breaking changes 🪚 are introduced: add the breaking change label
If bugs 🐞 are fixed: add the backport <release branch> label
The documentation website 📚 has received the love it deserves

github-actions · 2024-11-13T11:59:40Z

Affected libs: api-repository, feature-catalog, feature-record, feature-router, feature-editor, feature-search, feature-map, feature-dataviz, feature-auth, common-domain, api-metadata-converter, ui-search, common-fixtures, ui-elements, feature-notifications, ui-catalog, util-shared, ui-widgets, ui-inputs, ui-layout, ui-map, ui-dataviz,
Affected apps: metadata-editor, datahub, demo, webcomponents, map-viewer, search, datafeeder, metadata-converter, data-platform,

🚀 Build and deploy storybook and demo on GitHub Pages
📦 Build and push affected docker images

github-actions · 2024-11-13T12:06:17Z

📷 Screenshots are here!

jahow

Thanks! I did put some comments on what's already there, I'm not 100% sure what is the best way, let me know if you have better ideas!

...tor/src/app/dashboard/search-filters-summary-item/search-filters-summary-item.component.html

jahow · 2024-11-14T09:40:15Z

...ditor/src/app/dashboard/search-filters-summary-item/search-filters-summary-item.component.ts

+    const currentFieldValues: (FieldValue | DateRange)[] = await firstValueFrom(
+      this.fieldValues$
+    )
+    const updatedFieldValues = currentFieldValues.filter(
+      (value: string | DateRange) => value !== fieldValue
+    )
+    this.fieldsService
+      .buildFiltersFromFieldValues({
+        [this.fieldName]: updatedFieldValues as FieldValue[],
+      })
+      .subscribe((filters) => this.searchService.updateFilters(filters))


how about directly modifying the current filters instead of going from filters to values and then values to filters? simply deleting the corresponding key in the filters should be enough

I must be thinking too complicated, but what would this look like? An additional method in the service or facade to delete one filter by its key? Also, date ranges don't use their value as a key in the object.

Ok I looked into it and honestly I couldn't find a better way, so I think your approach makes sense. It ends up being verbose because we need to alternate between the "filters" model and the "values" model, but I guess this is something we cannot avoid...

...tadata-editor/src/app/dashboard/search-filters-summary/search-filters-summary.component.html

...metadata-editor/src/app/dashboard/search-filters-summary/search-filters-summary.component.ts

jahow · 2024-11-14T09:43:10Z

...metadata-editor/src/app/dashboard/search-filters-summary/search-filters-summary.component.ts

+  @Input() searchFields: string[] = []
+
+  searchFilterActive$ = this.searchFacade.searchFilters$.pipe(
+    map((filters) => this.hasNonEmptyValues(filters))
+  )


I'm wondering if instead of taking a list of filters as input we can simply listen for the search filters and show everyone of them. This means that if another filter (which does not appear in the advanced filters above) is active then it will also show up. What do you think?

I gave it a try, but had some difficulties with the fact that the field names do not always correspond to the ES field names in the filters. I'm also wondering if it is not more intuitive from a user perspective to only display the summary badges for the filters visible in the filter component above. For example, on the myRecords page the filter { owner: user.id } should always be present.

jahow · 2024-11-14T09:44:53Z

libs/feature/search/src/lib/utils/service/fields.service.ts

+marker('search.filters.summaryLabel.user')
+marker('search.filters.summaryLabel.changeDate')


how about marking labels for all fields? It might make more sense taken from outside I think. Another approach would be to use the field normal label if a summaryLabel is not available.

Right, I'll add more translations for this, I think.

Finally, I've added a fallback in 6b01ea9 to use the filters label, as we don't have the exact wording for more filter summary labels yet. Can be tested by adding any other filter to the allRecords component, for example.

tkohr · 2024-11-14T15:42:07Z

Thanks for the suggestions @jahow, I'll have a look how to refactor things this way.

jahow

Thank you, this looks really good! Just a few final comments but it works well in the app and I think the implementation makes a lot of sense now.

jahow · 2024-11-22T11:24:37Z

.../feature/search/src/lib/search-filters-summary-item/search-filters-summary-item.component.ts

+        case 'userInfo':
+          return { value, label: formatUserInfo(value) }


I think that instead of adding a very specific field type here we can simply look at the field name; if it is user then use this logic :)

jahow · 2024-11-22T11:26:33Z

libs/feature/search/src/lib/utils/service/fields.ts

+  getType(): FieldType {
+    return 'userInfo'


see comment above about not introducing a new field type

jahow · 2024-11-22T11:26:56Z

libs/ui/inputs/src/lib/badge/badge.component.html

    "
  >
-    <mat-icon class="material-symbols-outlined leading-[1.1]">close</mat-icon>
+    <mat-icon class="material-symbols-outlined">close</mat-icon>


hmm, we should't have mat-icons anymore :)

…s-summary-item

…s present

…omponent

…ar all

…method to simplify template and align badges

move one common formatUserInfo() to utils at the same time

…f no summary label defined

and use fieldname 'user' in component

treat dates in local timezone in app and without time in URL

Angi-Kinas

Thank you @tkohr, this will improve the usability of the filters quite a lot!

The code looks good to me.
While testing I had some behaviors that we might need to have a look at:

If we use user= as a query param, we get an empty badge
If we do the same with changeDate, we see an empty search filters summary component with the "reset" button

And as mentioned to you before, the filters are only working as expected on "Metadata records" but not on "My Records". I think this is more important than the issues mentioned above. When this is fixed, you can merge from my side 🚀

Angi-Kinas · 2024-12-03T14:37:41Z

libs/feature/router/src/lib/default/state/query-params.utils.ts

@@ -42,8 +44,8 @@ export function expandQueryParams(
      } else if (isDateUrl(value)) {
        const [start, end] = value.split('..')
        expanded[key] = {
-          ...(start && { start: new Date(start) }),
-          ...(end && { end: new Date(end) }),
+          ...(start && { start: new Date(`${start}T00:00:00`) }),


I guess T00:00:00 stands for the time... Is the time necessary here?

tkohr mentioned this pull request Nov 14, 2024

Editor: Add a date range search filter #1030

Open

10 tasks

jahow added this to the 2.4.0 milestone Nov 14, 2024

jahow reviewed Nov 14, 2024

View reviewed changes

jahow reviewed Nov 22, 2024

View reviewed changes

tkohr force-pushed the me-date-filter branch from 2693db5 to f95e16d Compare November 26, 2024 08:28

tkohr added 15 commits November 26, 2024 09:53

feat(search-filters): add search-filters-summary

e394c30

refactor(search-filters): refactor summary component

649312b

refactor(search-filters-summary): rename component into search-filter…

e24b22f

…s-summary-item

feat(search-filters): display summary and filter labels only if value…

65d581f

…s present

feat(search-filters): introduce intermediate search-filters-summary.c…

cb911c4

…omponent

feat(search-filter-summary): allow to remove one filter value and cle…

7f43c5d

…ar all

feat(search-filters-summary): adapt UI

f3697aa

refactor(search-filters-summary-item): implement a getReadableValues …

8a773bd

…method to simplify template and align badges

refactor(search-filter-summary): move components for feature-search lib

3eed1cb

fix(search-filters-summary): fix error when used in myRecords

5949728

feat(search-filter-summary): format userInfo in summary badges

526849e

move one common formatUserInfo() to utils at the same time

feat(search-filter-summary): provide filter label as fallback label i…

ec6a484

…f no summary label defined

refactor(fields): remove FieldType 'userInfo'

15b04d7

and use fieldname 'user' in component

test(search-filter-summary): add unit tests

1e110bc

rebase(search-filters): clean after rebase

2671c55

tkohr force-pushed the me-search-filters-summary branch from 5ff9f51 to 2671c55 Compare November 26, 2024 09:23

test(search-filter-summary): add e2e tests

bbe388a

tkohr marked this pull request as ready for review November 26, 2024 10:47

fix(dateRange/e2e): handle different timezones

ef2c59f

treat dates in local timezone in app and without time in URL

Angi-Kinas reviewed Dec 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editor: Add a search filters summary component #1037

Editor: Add a search filters summary component #1037

tkohr commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

jahow left a comment

jahow Nov 14, 2024

tkohr Nov 19, 2024

jahow Nov 22, 2024

jahow Nov 14, 2024

tkohr Nov 19, 2024

jahow Nov 14, 2024

tkohr Nov 19, 2024

tkohr Nov 21, 2024 •

edited

Loading

tkohr commented Nov 14, 2024

jahow left a comment

jahow Nov 22, 2024

jahow Nov 22, 2024

jahow Nov 22, 2024

Angi-Kinas left a comment •

edited

Loading

Angi-Kinas Dec 3, 2024

		marker('search.filters.summaryLabel.user')
		marker('search.filters.summaryLabel.changeDate')

		case 'userInfo':
		return { value, label: formatUserInfo(value) }

Editor: Add a search filters summary component #1037

Are you sure you want to change the base?

Editor: Add a search filters summary component #1037

Conversation

tkohr commented Nov 13, 2024 • edited Loading

Description

Screenshots

Quality Assurance Checklist

github-actions bot commented Nov 13, 2024 • edited Loading

github-actions bot commented Nov 13, 2024 • edited Loading

jahow 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkohr Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

tkohr commented Nov 14, 2024

jahow 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

Angi-Kinas left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkohr commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

tkohr Nov 21, 2024 •

edited

Loading

Angi-Kinas left a comment •

edited

Loading