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.

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

Use dynamic add actions for topology context menu #10100

Merged
merged 1 commit into from
Dec 7, 2021
Merged

Use dynamic add actions for topology context menu #10100

merged 1 commit into from
Dec 7, 2021

Conversation

rottencandy
Copy link
Contributor

@rottencandy rottencandy commented Sep 17, 2021 •
edited
Loading

Fixes:
https://issues.redhat.com/browse/ODC-6349

Description:
Use extension based Add actions instead of hard coded actions for topology context menu of add to project.

Screen shots / Gifs for design review:
1
2
3

Unit test coverage report:

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

/kind feature

@openshift-ci openshift-ci bot added component/dev-console Related to dev-console component/helm Related to helm-plugin component/knative Related to knative-plugin component/sdk Related to console-plugin-sdk component/topology Related to topology kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated labels Sep 20, 2021
@sahil143 sahil143 mentioned this pull request Sep 20, 2021
Merged
@rottencandy rottencandy force-pushed the feat/topology-dyn-actions branch 7 times, most recently from 8f3c7f4 to 77785b0 Compare September 21, 2021 17:43
@rottencandy rottencandy force-pushed the feat/topology-dyn-actions branch 2 times, most recently from 429bdf2 to ad97c2d Compare September 22, 2021 15:42
@debsmita1
Copy link
Contributor

Cannot see Helm charts , upload jar file , database options
Screenshot from 2021-09-24 00-42-13

@openshift-ci openshift-ci bot added the component/shared Related to console-shared label Sep 24, 2021
@rottencandy
Copy link
Contributor Author

Cannot see Helm charts , upload jar file , database options

Added upload jar file & serverless add options, I haven't added helm & database because they were not shown previously.
1

@debsmita1
Copy link
Contributor

Cannot see Helm charts , upload jar file , database options

Added upload jar file & serverless add options, I haven't added helm & database because they were not shown previously.
1

yes the knative add options were not showing up , I wrote jar file option instead . My bad.

@debsmita1
Copy link
Contributor

getting Maximum update depth exceeded warnings

@rottencandy
Copy link
Contributor Author

getting Maximum update depth exceeded warnings

Was happening because useAccessFilterExtensions was not fully memoized. Fixed.

@rottencandy
Copy link
Contributor Author

/cc @christianvogt

@rottencandy
Copy link
Contributor Author

/assign @rohitkrai03

@rottencandy rottencandy closed this Dec 1, 2021
@rottencandy rottencandy deleted the feat/topology-dyn-actions branch December 1, 2021 10:41
@rottencandy rottencandy restored the feat/topology-dyn-actions branch December 1, 2021 10:43
@rottencandy rottencandy reopened this Dec 1, 2021
@rohitkrai03
Copy link
Contributor

/retest

rohitkrai03
rohitkrai03 suggested changes Dec 1, 2021
View reviewed changes
Copy link
Contributor

@rohitkrai03 rohitkrai03 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@rottencandy Overall changes looks good. I found a few issues while running the PR on a cluster -

  • The in-context connection doesn't work anymore. The resource is created but the connection is not created.
  • The UI breaks when I click on the explicitly created connector between two resources.
Screen.Recording.2021-12-01.at.9.47.45.PM.mov

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2021
@rottencandy
Copy link
Contributor Author

@rottencandy Overall changes looks good. I found a few issues while running the PR on a cluster -

* The in-context connection doesn't work anymore. The resource is created but the connection is not created.

* The UI breaks when I click on the explicitly created connector between two resources.

Fixed in-context connection creation & connector sidebar crash.

@rottencandy
Copy link
Contributor Author

Propagating Docs & PX ack labels from epic ODC-6351
/label docs-approved
/label px-approved

@openshift-ci openshift-ci bot added docs-approved Signifies that Docs has signed off on this PR px-approved Signifies that Product Support has signed off on this PR labels Dec 3, 2021
@rohitkrai03
Copy link
Contributor

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 6, 2021 •
edited
Loading

@rottencandy: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/kubevirt-plugin 6004a35 link true /test kubevirt-plugin

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@rottencandy
Copy link
Contributor Author

/retest-required

rohitkrai03
rohitkrai03 approved these changes Dec 6, 2021
View reviewed changes
Copy link
Contributor

@rohitkrai03 rohitkrai03 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 6, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rohitkrai03, rottencandy, sahil143

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2021
@sahil143
Copy link
Contributor

sahil143 commented Dec 7, 2021

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Dec 7, 2021
@openshift-merge-robot openshift-merge-robot merged commit d2cf8af into openshift:master Dec 7, 2021
@spadgett spadgett added this to the v4.10 milestone Dec 8, 2021
@rottencandy rottencandy deleted the feat/topology-dyn-actions branch April 21, 2022 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@debsmita1 debsmita1 debsmita1 requested changes

@rohitkrai03 rohitkrai03 rohitkrai03 approved these changes

@sahil143 sahil143 sahil143 approved these changes

@christianvogt christianvogt Awaiting requested review from christianvogt

Assignees

@rohitkrai03 rohitkrai03

@sahil143 sahil143

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/dev-console Related to dev-console component/helm Related to helm-plugin component/knative Related to knative-plugin component/kubevirt Related to kubevirt-plugin component/sdk Related to console-plugin-sdk component/shared Related to console-shared component/topology Related to topology docs-approved Signifies that Docs has signed off on this PR kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Milestone
v4.10
Development

Successfully merging this pull request may close these issues.
6 participants
@rottencandy @debsmita1 @sahil143 @rohitkrai03 @spadgett @openshift-merge-robot