Skip to content

HorizontalNavTab Extension #3917

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Jan 29, 2020
Merged

HorizontalNavTab Extension #3917

openshift-merge-robot merged 1 commit into from
Jan 29, 2020

Conversation

@alecmerdler
Copy link
Contributor

@alecmerdler alecmerdler commented Jan 10, 2020 •
edited
Loading

Description

Implements the DetailViewTab extension type, which allows adding a new tab to an existing resource's detail view.

Screenshots

Pods detail view with demo plugin:
Screenshot_20200109_164724

Needed for https://issues.redhat.com/browse/PROJQUAY-119
Blocked by #3383

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 10, 2020
@openshift-ci-robot openshift-ci-robot added component/core Related to console core functionality component/sdk Related to console-plugin-sdk labels Jan 10, 2020
@alecmerdler alecmerdler force-pushed the detail-view-tab-extension branch from 24f2128 to 370b466 Compare January 10, 2020 00:55
@openshift-ci-robot openshift-ci-robot added the component/olm Related to OLM label Jan 10, 2020
christianvogt
christianvogt reviewed Jan 10, 2020
View reviewed changes
vojtechszocs
vojtechszocs suggested changes Jan 10, 2020
View reviewed changes
frontend/packages/console-plugin-sdk/src/typings/details.ts Outdated
Copy link
Contributor

@vojtechszocs vojtechszocs Jan 10, 2020

Choose a reason for hiding this comment

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

@alecmerdler This is effectively about extending the common HorizontalNav component which is used on many places. If I read the code correctly, this covers more than just resource details pages.

I'd suggest to give this extension a more accurate name. For example, HorizontalNavTab as extension type name and HorizontalNav/Tab as type property value.

In general, the type value should reflect the context of the extension. For example, Dashboards/Tab means a tab on dashboards page, and Dashboards/Overview/Inventory/Item means new item in inventory which is part of overview on dashboards page.

@alecmerdler alecmerdler changed the title DetailViewTab Extension [WIP] DetailViewTab Extension Jan 10, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 10, 2020
@alecmerdler alecmerdler force-pushed the detail-view-tab-extension branch from 370b466 to 2f94c23 Compare January 10, 2020 22:32
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. component/dev-console Related to dev-console and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 10, 2020
@alecmerdler alecmerdler mentioned this pull request Jan 10, 2020
Merged
@alecmerdler alecmerdler force-pushed the detail-view-tab-extension branch 5 times, most recently from 97afe54 to 3164920 Compare January 23, 2020 23:31
@openshift-ci-robot openshift-ci-robot added the component/shared Related to console-shared label Jan 23, 2020
@spadgett spadgett mentioned this pull request Jan 24, 2020
Merged
alecmerdler
alecmerdler commented Jan 24, 2020
View reviewed changes
frontend/packages/console-demo-plugin/src/plugin.tsx Outdated
Copy link
Contributor Author

@alecmerdler alecmerdler Jan 24, 2020

Choose a reason for hiding this comment

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

NOTE: It appears that the @console/demo-plugin is broken (circular dependency issue). But I've tested this plugin using the planned feature addition of #3874.

Copy link
Contributor

@vojtechszocs vojtechszocs Jan 24, 2020

Choose a reason for hiding this comment

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

Thanks for the heads-up, I will try to look into it.

Regardless, thanks @alecmerdler for keeping the demo plugin in mind when adding new extension type.

@alecmerdler alecmerdler force-pushed the detail-view-tab-extension branch from 3164920 to 84c43df Compare January 24, 2020 01:07
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 24, 2020
@alecmerdler alecmerdler changed the title [WIP] DetailViewTab Extension DetailViewTab Extension Jan 24, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 24, 2020
@alecmerdler
Copy link
Contributor Author

alecmerdler commented Jan 24, 2020 •
edited
Loading

@vojtechszocs @christianvogt @spadgett Rebased and using the new useExtensions React hook. I believe I addressed all comments. Please take a look when you have a chance.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

5 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 25, 2020
@openshift openshift deleted a comment from openshift-bot Jan 25, 2020
@openshift openshift deleted a comment from openshift-bot Jan 25, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2020
@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

16 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 28, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 29, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 29, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 29, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 29, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 29, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 29, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit beac546 into openshift:master Jan 29, 2020
@spadgett spadgett added this to the v4.4 milestone Feb 4, 2020
@alecmerdler alecmerdler mentioned this pull request Apr 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kyoto kyoto Awaiting requested review from kyoto

@spadgett spadgett Awaiting requested review from spadgett

2 more reviewers

@christianvogt christianvogt christianvogt left review comments

@vojtechszocs vojtechszocs vojtechszocs requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vojtechszocs vojtechszocs

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dev-console Related to dev-console component/olm Related to OLM component/sdk Related to console-plugin-sdk component/shared Related to console-shared lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

v4.4

Development

Successfully merging this pull request may close these issues.

7 participants

@alecmerdler @vojtechszocs @openshift-ci-robot @spadgett @openshift-bot @christianvogt @openshift-merge-robot