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

fix(uss/datasets): Reset VSCode multi-select flag after Hide Profile in multi-selection #2108

Merged
merged 6 commits into from
Feb 7, 2023

Conversation

traeok
Copy link
Member

@traeok traeok commented Feb 1, 2023

Proposed changes

This should resolve the bug where VSCode still thinks there is a multi-selection active, even though the nodes in the previous selection were deleted. This typically happens when a user uses "Hide Profile" in a multi-select operation, as it removes the nodes that were previously a part of that selection.

Release Notes

Milestone: 2.6.1

Changelog:

  • Resolves an issue where VSCode did not provide all context menu options for a profile node.

Types of changes

What types of changes does your code introduce to Zowe Explorer?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)

Checklist

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 reviewer

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • PR title follows Conventional Commits Guidelines
  • PR Description is included
  • gif or screenshot is included if visual changes are made
  • yarn workspace vscode-extension-for-zowe vscode:prepublish has been executed
  • All checks have passed (DCO, Jenkins and Code Coverage)
  • I have added unit test and it is passing
  • I have added integration test and it is passing
  • There is coverage for the code that I have added
  • I have tested it manually and there are no regressions found

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@traeok traeok changed the base branch from main to maintenance February 1, 2023 21:34
@codecov
Copy link

codecov bot commented Feb 1, 2023

Codecov Report

Base: 72.68% // Head: 72.83% // Increases project coverage by +0.15% 🎉

Coverage data is based on head (9b5deca) compared to base (a0b5c75).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 9b5deca differs from pull request most recent head a1ed6ee. Consider uploading reports for the commit a1ed6ee to get more accurate results

Additional details and impacted files
@@               Coverage Diff               @@
##           maintenance    #2108      +/-   ##
===============================================
+ Coverage        72.68%   72.83%   +0.15%     
===============================================
  Files               76       77       +1     
  Lines             8039     8047       +8     
  Branches          1706     1706              
===============================================
+ Hits              5843     5861      +18     
+ Misses            2196     2186      -10     
Impacted Files Coverage Δ
packages/zowe-explorer/src/extension.ts 48.38% <100.00%> (+2.49%) ⬆️
packages/zowe-explorer/src/utils/TreeViewUtils.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@traeok traeok force-pushed the fix/hide-profile-multi-select branch from 837ab04 to f48cf8c Compare February 2, 2023 21:14
@traeok traeok marked this pull request as ready for review February 3, 2023 14:27
@traeok traeok added this to the v2.6.1 milestone Feb 3, 2023
@traeok traeok added the no-changelog Add to PR's that don't require a CHANGELOG update label Feb 6, 2023
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

It seems like this works fine for USS and Jobs, but I'm seeing some strange behavior for Datasets 😢
Here is a quick GIF:
temp

@traeok
Copy link
Member Author

traeok commented Feb 6, 2023

It seems like this works fine for USS and Jobs, but I'm seeing some strange behavior for Datasets 😢 Here is a quick GIF: temp

🤯 oh wow, that was unexpected. Maybe this is related to #2098 ? I tried the same steps but cannot replicate this problem when building from the branch. I also tried this w/ the VSIX and the behavior is working as expected in my environment.

@zFernand0
Copy link
Member

zFernand0 commented Feb 6, 2023

🤯 oh wow, that was unexpected. Maybe this is related to #2098 ? I tried the same steps but cannot replicate this problem when building from the branch. I also tried this w/ the VSIX and the behavior is working as expected in my environment.

It might be related 😱
I'm happy to test both PRs combined if you would like me to do so! 😋
Otherwise, we can sync-up offline 😉


UPDATE:
It is related.
Testing both PRs together fixes the problem for me 💯

@traeok
Copy link
Member Author

traeok commented Feb 6, 2023

UPDATE: It is related. Testing both PRs together fixes the problem for me 💯

Great, thanks for confirming @zFernand0 !

zFernand0
zFernand0 previously approved these changes Feb 6, 2023
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

My label duplication was resolved after testing with #2098.
This PR LGTM! 😋

@traeok traeok removed the no-changelog Add to PR's that don't require a CHANGELOG update label Feb 6, 2023
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
@traeok traeok force-pushed the fix/hide-profile-multi-select branch from f0808ad to a432790 Compare February 6, 2023 19:26
@traeok traeok requested a review from zFernand0 February 6, 2023 19:40
zFernand0
zFernand0 previously approved these changes Feb 6, 2023
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

Thanks for adding the changelog!
LGTM! 😋

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM! 😋
Thanks for fixing the merge conflicts! 🥳

@sonarcloud
Copy link

sonarcloud bot commented Feb 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@zFernand0 zFernand0 merged commit 5d2aea0 into maintenance Feb 7, 2023
@zFernand0 zFernand0 deleted the fix/hide-profile-multi-select branch February 7, 2023 15:03
@traeok traeok linked an issue Feb 8, 2023 that may be closed by this pull request
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.

Profile context menus get stuck in multi-select mode
3 participants