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

feat: Jobs tabular view #3050

Merged
merged 44 commits into from
Aug 20, 2024
Merged

feat: Jobs tabular view #3050

merged 44 commits into from
Aug 20, 2024

Conversation

traeok
Copy link
Member

@traeok traeok commented Aug 14, 2024

Proposed changes

  • Jobs table view implementation (using the new table facilities)
  • Added a toggle columns section w/ search ("gear" button) to the tables
  • Added support for codicons in all of our webviews 😋

Release Notes

Milestone:

Changelog:

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds or improves functionality)
  • Breaking change (a change that would cause existing functionality to not work as expected)
  • Documentation (Markdown, README updates)
  • Other (please specify above in "Proposed changes" section)

Checklist

General

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • All PR dependencies have been merged and published (if applicable)
  • A GIF or screenshot is included in the PR for visual changes
  • The pre-publish command has been executed:
    • v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
    • v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

  • There is coverage for the code that I have added
  • I have added new test cases and they are passing
  • I have manually tested the changes

Deployment

  • I have added developer documentation (if applicable)
  • Documentation should be added to Zowe Docs
    • User doc reviewed by Ana; will be uploaded as part of v3 doc content
  • These changes may need ported to the appropriate branches (list here):

traeok added 14 commits August 9, 2024 14:29
Signed-off-by: Trae Yelovich <[email protected]>
Copy link

codecov bot commented Aug 14, 2024

Codecov Report

Attention: Patch coverage is 98.79518% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.94%. Comparing base (97b157e) to head (13e74b3).
Report is 45 commits behind head on next.

Files Patch % Lines
...ckages/zowe-explorer/src/trees/job/JobTableView.ts 98.52% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next    #3050      +/-   ##
==========================================
+ Coverage   92.89%   92.94%   +0.04%     
==========================================
  Files         111      112       +1     
  Lines       11180    11261      +81     
  Branches     2362     2433      +71     
==========================================
+ Hits        10386    10466      +80     
- Misses        792      793       +1     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@traeok traeok marked this pull request as ready for review August 15, 2024 15:05
@traeok traeok changed the title [WIP] feat: Jobs tabular view feat: Jobs tabular view Aug 15, 2024
@t1m0thyj t1m0thyj self-requested a review August 19, 2024 20:21
zFernand0
zFernand0 previously approved these changes Aug 20, 2024
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! 😋

My comments were mostly questions and nothing that should impact end-user behavior.

I learned quite a bit while reviewing this PR! 🙏🏽

Nicely done! 🥳

// Invoke the wrapped function once to get the built function, then invoke it again with the parameters
let shouldDisable = selectionCount === 0;
if (cond != null) {
shouldDisable ||= !cond()(action.callback.typ === "multi-row" ? selectedRows : selectedRows[0]);
Copy link
Member

Choose a reason for hiding this comment

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

I might be misinterpreting something here but... is this a typo?
action.callback.typ -> action.callback.type?

Copy link
Member Author

Choose a reason for hiding this comment

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

type is a reserved keyword so I used the shorthand as a way to define a "type" property 😅
This could probably be renamed in the future but since it the framework was already merged with typ as the property name, I'm hesitant to make the change

t1m0thyj
t1m0thyj previously approved these changes Aug 20, 2024
Copy link
Member

@t1m0thyj t1m0thyj 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 @traeok!

@traeok traeok dismissed stale reviews from t1m0thyj and zFernand0 via b07a19a August 20, 2024 14:43
@traeok traeok requested review from t1m0thyj and zFernand0 August 20, 2024 14:44
zFernand0
zFernand0 previously approved these changes Aug 20, 2024
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 addressing the feedback so quickly! 🥳

Copy link

Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

❤️ the auto-sizing, thanks @traeok!

@JillieBeanSim JillieBeanSim merged commit 7ff0298 into next Aug 20, 2024
28 checks passed
@JillieBeanSim JillieBeanSim deleted the rnd/tables branch August 20, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

5 participants