-
Notifications
You must be signed in to change notification settings - Fork 636
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
Pm search for packages - filters #14322
Conversation
- moving 'SearchForPackages' PR into smaller PRs - main chunk of work on the PackageManagerSearchControl -- context menu -- filter tags
- added all filters to the GetAllPackages routine
- removed the `active` filter - introduced `new` and `updated` as status filters
- added 'clear all' to resources
- no selected package when opening the package manager for the first time
- correctly assigned the `clear all` button text
- added tests for the new filters
- standardized ui element styles
- the user will be able to select multiple filters without the context menu closing
src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs
Show resolved
Hide resolved
src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs
Outdated
Show resolved
Hide resolved
- the two dependency filters (package 'has decency' and 'has no dependency') cannot be both in positive states - adjusted tests to take that change into account
src/DynamoCoreWpf/Views/PackageManager/Controls/FilterTagControl.xaml.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments then LGTM, also some merge conflicts because the version PR merged today
- added tooltips for all new filters, - updated old filters, - changed styling
- added comments for the public properties of the FilterTagControl
src/DynamoCoreWpf/Views/PackageManager/Controls/FilterTagControl.xaml.cs
Outdated
Show resolved
Hide resolved
/// `Has Dependency` and `Has No Dependency` filters are mutually exclusive | ||
/// </summary> | ||
[Test] | ||
public void PackageSearchDialogSearchTestDependencyFilters() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dnenov Thank you for covering these tests.
src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs
Show resolved
Hide resolved
- replaced hard-coded string value with nameof property
//private bool showDeprecated; | ||
//private bool showDependencies; | ||
//private bool showNoDependencies; | ||
//private bool isAnyFilterOn; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this commented code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, absolutely, sorry for the mess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one minor comment left. Also @dnenov can you remind me if we covered the fix that first time launching Dynamo PM the first package entry is always highlighted somewhere?
Purpose
Continuation of the work done on the Package Manager. This PR focuses on the implementation of new Filters in the first tab of the PM - the Search for Packages tab.
Filter tag bubbles added to the UI with the expected functionality.
UI Changes
Declarations
Check these if you believe they are true
*.resx
filesRelease Notes
Sorting changed - default priority now to 'votesreverted back to last updateFilterTag
bubble custom control createdclear all
button clears all currently applied filtersFilterEntity
between host and non-host filters (reusing the logic of the existing host filter VM as much as possible)has dependency
andhas no dependency
filters toggle each other out, if positive (they are mutually exclusive)Reviewers
@QilongTang
@reddyashish
FYIs
@Amoursol