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

Added a new filter-side-drawer component for filtering reports #390

Merged
merged 36 commits into from
May 7, 2024

Conversation

MarkvdVorst
Copy link
Contributor

Pull Request 231 of the Ladybug project should be accepted before accepting this pull request.

Added a filter side drawer that replaces the old filter feature.
The new filter component now communicates with a filter service to send and receive the filters of the table.

Most of the functionality in the table component remains the same to filter the table. Parts that require the input of the filter however have been changed so that it now communicates with the filter service.

This feature also solves issue 290; When multiple filters are filled only one is used.

… needs to be aligned correctly including other elements in the filter-side-drawer
…e the basic method calls for updating the filter. it just needs to call the updatefilters method in table component now
…vice.ts

table component functionality has barely been changed. only difference is that the api request now accepts string arrays so the parameters for the api call have also been changed.
@MarkvdVorst MarkvdVorst self-assigned this Apr 23, 2024
@MarkvdVorst MarkvdVorst linked an issue Apr 23, 2024 that may be closed by this pull request
# Conflicts:
#	src/app/app.module.ts
#	src/app/debug/table/table.component.ts
…html

Co-authored-by: Sergi Philipsen <philipsen.sergi@gmail.com>
removed resetFilter from filter-side-drawer.component.ts as the trigger might as well call the resetFitler directly in the filterService.
updateFilter parameter type added.
Added autocomplete feature for filter options. Fixes a known ladybug-frontend issue
Fixed filter side drawer size option. now possible to scroll if screen is too small
@MarkvdVorst MarkvdVorst linked an issue Apr 27, 2024 that may be closed by this pull request
MarkvdVorst and others added 5 commits May 2, 2024 15:40
subscriptions moved to dedicated subscribe and unsubscribe methods.
Records have been changed to Maps for consistency.
small css changes.
Added a type to one of the subscribers where I forgot to add one.
made filter-side-drawer.component.ts standalone
Changed ngIf to an @if in filter-side-drawer.component.html
Replaced the empty mat-option with a clear button that shows next to the metadata input element.
updated the cypress e2e test accordingly with the new clear button
Co-authored-by: Matthijs Smets <93487259+MatthijsSmets@users.noreply.github.com>
@MarkvdVorst MarkvdVorst requested a review from MatthijsSmets May 7, 2024 07:43
Copy link
Contributor

@MatthijsSmets MatthijsSmets left a comment

Choose a reason for hiding this comment

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

Almost there!

MarkvdVorst and others added 3 commits May 7, 2024 12:29
…html

Co-authored-by: Matthijs Smets <93487259+MatthijsSmets@users.noreply.github.com>
…html

Co-authored-by: Matthijs Smets <93487259+MatthijsSmets@users.noreply.github.com>
@MarkvdVorst MarkvdVorst requested a review from MatthijsSmets May 7, 2024 10:35
MatthijsSmets
MatthijsSmets previously approved these changes May 7, 2024
@MarkvdVorst
Copy link
Contributor Author

@Matthbo @philipsens Does anything else need to be changed before merge?

@philipsens
Copy link
Member

I don't have any additional feedback.

Copy link
Member

@Matthbo Matthbo left a comment

Choose a reason for hiding this comment

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

Looks good, you could've bundled all the subscriptions on a component to one bug subscription variable that you can easily .unsubscribe() in ngOnDestroy instead of doing it for each subscription on its own.
But this works as well so there is no need to do it now

@MarkvdVorst MarkvdVorst merged commit 5a64ad7 into master May 7, 2024
1 check passed
@MarkvdVorst MarkvdVorst deleted the feature/filter-side-drawer branch May 7, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Filter does not always update the report table When multiple filters are filled only one is used
4 participants