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

[Datahub] Add search filter for organisations #717

Merged
merged 15 commits into from
Dec 18, 2023
Merged

[Datahub] Add search filter for organisations #717

merged 15 commits into from
Dec 18, 2023

Conversation

tkohr
Copy link
Collaborator

@tkohr tkohr commented Dec 8, 2023

PR adds a search filter for the organisations:

Todos:

  • finish search-input: display clear only if value, no button for search icon, remove red background color, etc.
  • rename sort to filter component ?
  • i18n (lots of translations seem out of sync)
  • search user experience: should we really always apply OR filter on inputs? => impossible to narrow search down to one result => changed to AND after discussion
  • clarify why emails of orgs are erased by groups mapping => not sure if my fix is correct here => removed search in emails after discussion
  • tests

Funded by SwissTopo geocat.ch

search_filter_orgs

Copy link
Contributor

github-actions bot commented Dec 8, 2023

Affected libs: feature-catalog, feature-record, feature-router, ui-catalog, ui-inputs, feature-dataviz, feature-search, feature-map, feature-editor, ui-elements, ui-search,
Affected apps: metadata-editor, datahub, demo, webcomponents, search, metadata-converter, map-viewer, datafeeder,

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

@fgravin fgravin linked an issue Dec 12, 2023 that may be closed by this pull request
@fgravin fgravin changed the title Datahub: Add search filter for organisations [SwissTopo] [Datahub] Add search filter for organisations Dec 12, 2023
@tkohr tkohr marked this pull request as ready for review December 14, 2023 16:26
@fgravin fgravin changed the title [SwissTopo] [Datahub] Add search filter for organisations [Datahub] Add search filter for organisations Dec 14, 2023
@coveralls
Copy link

Coverage Status

coverage: 81.942% (-2.0%) from 83.923%
when pulling a4947b1 on orgs-filter
into 92348fa on main.

Copy link
Member

@fgravin fgravin left a comment

Choose a reason for hiding this comment

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

Excellent, I start to like the dynamic, clear UX and sharp implementation.
I am wondering if we should have a number of char limit, like 2 or 3 chars.

You just missed one thing, the margin between the search info and the results. It should be bigger, and aligned with the dataset tab.

👌

@@ -33,8 +33,14 @@ export class OrganisationsSortComponent {
},
]
@Output() sortBy = new EventEmitter<SortByField>()
@Output() filterByValueChange = new EventEmitter<string>()
Copy link
Member

Choose a reason for hiding this comment

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

Is it really it an output ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah good catch, completely forgot about it when refactoring


selectOrderToDisplay(selectValue: string) {
this.sortBy.emit(selectValue.split(',') as SortByField)
}

filterOrganisations(inputValue: string) {
Copy link
Member

Choose a reason for hiding this comment

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

Note that the children output is an Observable, which you could reach with a @ChildView component ref.
Parent output would just be childrenOutput.pipe(debounce(300), no event handling with the component, no event emission, just propagation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting idea! I guess I'd need an ngOnInit for this to update the ouput, though, no?

As doing the following, the @Output()'s initialization accesses too early the searchInput:

@ViewChild('search') searchInput
@Output() filterBy = this.searchInput.valueChange.pipe(debounceTime(300))

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the ViewChild are only accessible afterViewInit.

@tkohr
Copy link
Collaborator Author

tkohr commented Dec 18, 2023

Thanks for the review @fgravin ! I did some testing introducing a minimum number of chars, but as we don't launch requests here, I finally decided against it. This way the user immediately gets a feedback that something is happening, when he/she stops typing, which seems more intuitive to me.

@fgravin fgravin merged commit 19be416 into main Dec 18, 2023
8 checks passed
@fgravin fgravin deleted the orgs-filter branch December 18, 2023 16:23
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.

[SwissTopo] [Datahub] Add a search filter in Organization panel
3 participants