-
Notifications
You must be signed in to change notification settings - Fork 92
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
fix(ds/sort): Fallback to sorting by name when values are equal #2538
Conversation
Signed-off-by: Trae Yelovich <[email protected]>
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.
Seems like this would work.
testing briefly before approving.
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. Thanks Trae!
…likely, but possible) Signed-off-by: Trae Yelovich <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
... and 2 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
One minor issue I am seeing that may be related to this is that if a member has no user ID, and you sort by User ID in the descending order, the member without a user ID will switch to the top and bottom of the list when you collapse and expand the PDS. |
Signed-off-by: Trae Yelovich <[email protected]>
db82aac
to
bc94caf
Compare
Signed-off-by: Trae Yelovich <[email protected]>
Thanks for catching this @adam-wolfe, I've addressed this in 9235f28 |
Signed-off-by: Trae Yelovich <[email protected]>
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.
Looks good.
Signed-off-by: Trae Yelovich <[email protected]>
I found a separate, unrelated issue with how we were building children in |
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! 😋
Thanks for addressing the comments
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
If we would prefer to keep this for a potential 2.12.1 release, then I will add a changelog. Otherwise, I think we can keep this as
no-changelog
as the feature will be released with 2.12.Proposed changes
I've cleaned up the sorting function a bit so that it falls back to sorting by name when 2 node values are equal.
In addition, I've adjusted the date checks slightly to also use Day.js (consistent with the rest of the date processing for this feature) for easier date comparison.
Release Notes
Milestone: 2.12.0 (or 2.12.1)
Changelog:
Types of changes
What types of changes does your code introduce to Zowe Explorer?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the revieweryarn workspace vscode-extension-for-zowe vscode:prepublish
has been executed