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

Clear all filtering options when class selection is edited #371

Merged
merged 14 commits into from Jan 8, 2024

Conversation

ViliusBa
Copy link
Contributor

Clear all filtering options when selected class list is changed.

@ViliusBa ViliusBa requested a review from a team as a code owner December 22, 2023 15:02
@ViliusBa
Copy link
Contributor Author

One use case scenario that concerns me:

  • User selects classes.
  • User adds property filtering rules.
  • User adds an additional class that supports all the properties that are currently in the filter.
  • Filtering rules are cleared.

It seems to me that clearing all filtering property rules every time selected class list changes might not be the behavior that an user expects.

Copy link

github-actions bot commented Dec 27, 2023

Benchmark

Benchmark suite Current: c05be4b Previous: 20fbd7d Deviation Status
Initial Models Tree Load: Baytown.bim 373 ms 339 ms 10.0295% 🚨
Full Models Tree Load: Baytown.bim 1799 ms 1860 ms -3.2796%

This comment was automatically generated by workflow using github-action-benchmark.

@ViliusBa ViliusBa changed the title Clear filtering option when class selection is edited Clear all filtering options when class selection is edited Dec 28, 2023
@grigasp grigasp linked an issue Jan 2, 2024 that may be closed by this pull request
@grigasp
Copy link
Member

grigasp commented Jan 2, 2024

One use case scenario that concerns me:

  • User selects classes.
  • User adds property filtering rules.
  • User adds an additional class that supports all the properties that are currently in the filter.
  • Filtering rules are cleared.

It seems to me that clearing all filtering property rules every time selected class list changes might not be the behavior that an user expects.

I believe we discussed that a warning / confirmation from user is needed before clearing the properties filter on a class list change. Maybe we could also clear the property filter only when a class is removed, but keep it if a class is added? @saskliutas, do you see anything wrong with that?

@ViliusBa ViliusBa merged commit d8d086b into master Jan 8, 2024
10 checks passed
@ViliusBa ViliusBa deleted the filtering-dialog/clear-options-onClassesChanged branch January 8, 2024 11:15
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.

Purpose of the class selector is unclear
3 participants