Update Event Sources Visualization and SideBar #10721

debsmita1 · 2021-12-22T18:35:34Z

Jira ticket:
https://issues.redhat.com/browse/ODC-6446

Solution description:

added 'sign-out-alt' icon to the event source node
On SideBar, 'Sink URI' Heading is replaced with 'Target URI'
On SideBar, 'Sink' Heading is replaced with 'Input Target'
On create Event Source form 'Sink' Section Heading is updated to 'Input Target' and the description is changed to "Add an input target to route this Event source to a Channel, Broker, Knative Service or another route."

Screenshots:

debsmita1 · 2021-12-22T18:35:52Z

/kind feature

debsmita1 · 2021-12-22T18:36:30Z

/cc @sahil143

sahil143

Looks good overall but the icon is not properly centered and the white circle is also not properly aligned

debsmita1 · 2021-12-28T18:12:07Z

Looks good overall but the icon is not properly centered and the white circle is also not properly aligned

Fixed it. PTAL

invincibleJai · 2021-12-29T08:00:42Z

frontend/packages/topology/src/components/svg/SvgBoxedText.tsx

@@ -45,6 +46,7 @@ const SvgBoxedText: React.FC<SvgBoxedTextProps> = ({
  y = 0,
  kind,
  typeIconClass,
+  typeIcon,


can't we use the existing prop we have here i.e typeIconClass instead of introducing a new one?

cc @sahil143

typeIconClass accepts svg & in this case, we want to pass the react element

invincibleJai · 2021-12-30T09:41:30Z

frontend/packages/topology/src/components/svg/SvgBoxedText.tsx

@@ -83,13 +92,14 @@ const SvgBoxedText: React.FC<SvgBoxedTextProps> = ({
          kind={kind}
        />
      )}
-      {textSize && iconSize && typeIconClass && (
+      {textSize && iconSize && (typeIconClass || typeIcon) && (


we can have this in const and avoid repeating typeIconClass || typeIcon

invincibleJai · 2021-12-30T09:44:12Z

frontend/packages/topology/src/components/svg/SvgCircledIcon.tsx

+        <g ref={typedIconRef}>
+          <foreignObject
+            key={`image-${FILTER_ID}`}
+            x={-iconWidth}


just verifying this change is needed right as we below https://github.com/openshift/console/pull/10721/files#diff-d1a68270878fe2df06bb4a025e09188cf6693875dae259d2f912f5702349a7adR59

x={x - iconWidth}

Did x={x - iconWidth + 3}

invincibleJai · 2021-12-30T11:24:12Z

@debsmita1 outline can be seen on the newly added icon but not on the existing one i.e

observed on mac, chrome

will be good to have outline: none

and just confirm if we need to show a tooltip here i.e Input Target

invincibleJai · 2021-12-30T11:34:31Z

another observation is icon is shown for ksvc, helm in list view as well which is not reflected in case of event sources

debsmita1 · 2021-12-30T13:24:36Z

another observation is icon is shown for ksvc, helm in list view as well which is not reflected in case of event sources

Done

vikram-raj · 2021-12-30T13:45:25Z

frontend/packages/knative-plugin/src/topology/listView/NoStatusListViewNode.tsx

+    <TopologyListViewNode
+      noPods
+      {...props}
+      badgeCell={props.item.getType() === 'event-source' ? badgeCell : null}


Can we make use of NodeType enum instead of hardcoding event-source?

Suggested change

badgeCell={props.item.getType() === 'event-source' ? badgeCell : null}

badgeCell={props.item.getType() === NodeType.EventSource ? badgeCell : null}

debsmita1 · 2021-12-30T13:52:04Z

and just confirm if we need to show a tooltip here i.e Input Target

Removed the tooltip

vikram-raj · 2021-12-30T13:55:59Z

@debsmita1 outline can be seen on the newly added icon but not on the existing one i.e

@invincibleJai I am not seeing an outline on the icon. I am on Mac and Chrome Version 96.0.4664.110. I checked it by clicking on icon. Is there any other way to reproduce it?

vikram-raj

Verified it and works as expected.

/lgtm

invincibleJai · 2022-01-04T06:22:43Z

/approve

Thanks have verified the changes, works as expected.

sahil143

/lgtm
/label qe-approved

openshift-ci · 2022-01-05T07:18:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: debsmita1, invincibleJai, sahil143, vikram-raj

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:

~~frontend/packages/knative-plugin/OWNERS~~ [invincibleJai]
~~frontend/packages/topology/OWNERS~~ [invincibleJai]

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

debsmita1 · 2022-01-05T07:20:02Z

/label docs-approved
/label px-approved

vikram-raj · 2022-01-05T07:22:36Z

Verified it on cluster bot. And it works as expected.

/label qe-approved

openshift-ci · 2022-01-05T11:41:22Z

@debsmita1: all tests passed!

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.

openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 22, 2021

openshift-ci bot requested review from abhinandan13jan and invincibleJai December 22, 2021 18:36

openshift-ci bot added component/knative Related to knative-plugin component/topology Related to topology kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated labels Dec 22, 2021

openshift-ci bot requested a review from sahil143 December 22, 2021 18:36

sahil143 suggested changes Dec 27, 2021

View reviewed changes

debsmita1 force-pushed the es-targetinput branch from ff76ac2 to c43a934 Compare December 28, 2021 18:11

debsmita1 requested a review from sahil143 December 28, 2021 18:12

invincibleJai reviewed Dec 29, 2021

View reviewed changes

invincibleJai reviewed Dec 30, 2021

View reviewed changes

debsmita1 force-pushed the es-targetinput branch from c43a934 to 884c102 Compare December 30, 2021 09:55

debsmita1 force-pushed the es-targetinput branch from 884c102 to 8cfcf4b Compare December 30, 2021 13:21

vikram-raj reviewed Dec 30, 2021

View reviewed changes

debsmita1 force-pushed the es-targetinput branch from 8cfcf4b to 270e94c Compare December 30, 2021 13:48

Update Event Sources Visualization and SideBar

7df491c

debsmita1 force-pushed the es-targetinput branch from 270e94c to 7df491c Compare December 30, 2021 15:16

vikram-raj reviewed Dec 30, 2021

View reviewed changes

openshift-ci bot assigned vikram-raj Dec 30, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 30, 2021

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 4, 2022

sahil143 approved these changes Jan 5, 2022

View reviewed changes

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jan 5, 2022

openshift-ci bot assigned sahil143 Jan 5, 2022

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 Jan 5, 2022

openshift-merge-robot merged commit 30f819d into openshift:master Jan 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Event Sources Visualization and SideBar #10721

Update Event Sources Visualization and SideBar #10721

debsmita1 commented Dec 22, 2021 •

edited

Loading

debsmita1 commented Dec 22, 2021

debsmita1 commented Dec 22, 2021

sahil143 left a comment

debsmita1 commented Dec 28, 2021

invincibleJai Dec 29, 2021

debsmita1 Dec 30, 2021

invincibleJai Dec 30, 2021

invincibleJai Dec 30, 2021

debsmita1 Dec 30, 2021

invincibleJai commented Dec 30, 2021 •

edited

Loading

invincibleJai commented Dec 30, 2021

debsmita1 commented Dec 30, 2021

vikram-raj Dec 30, 2021

debsmita1 commented Dec 30, 2021

vikram-raj commented Dec 30, 2021

vikram-raj left a comment

invincibleJai commented Jan 4, 2022

sahil143 left a comment

openshift-ci bot commented Jan 5, 2022

debsmita1 commented Jan 5, 2022

vikram-raj commented Jan 5, 2022

openshift-ci bot commented Jan 5, 2022

	badgeCell={props.item.getType() === 'event-source' ? badgeCell : null}
	badgeCell={props.item.getType() === NodeType.EventSource ? badgeCell : null}

Update Event Sources Visualization and SideBar #10721

Update Event Sources Visualization and SideBar #10721

Conversation

debsmita1 commented Dec 22, 2021 • edited Loading

debsmita1 commented Dec 22, 2021

debsmita1 commented Dec 22, 2021

sahil143 left a comment

Choose a reason for hiding this comment

debsmita1 commented Dec 28, 2021

invincibleJai Dec 29, 2021

Choose a reason for hiding this comment

debsmita1 Dec 30, 2021

Choose a reason for hiding this comment

invincibleJai Dec 30, 2021

Choose a reason for hiding this comment

invincibleJai Dec 30, 2021

Choose a reason for hiding this comment

debsmita1 Dec 30, 2021

Choose a reason for hiding this comment

invincibleJai commented Dec 30, 2021 • edited Loading

invincibleJai commented Dec 30, 2021

debsmita1 commented Dec 30, 2021

vikram-raj Dec 30, 2021

Choose a reason for hiding this comment

debsmita1 commented Dec 30, 2021

vikram-raj commented Dec 30, 2021

vikram-raj left a comment

Choose a reason for hiding this comment

invincibleJai commented Jan 4, 2022

sahil143 left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jan 5, 2022

debsmita1 commented Jan 5, 2022

vikram-raj commented Jan 5, 2022

openshift-ci bot commented Jan 5, 2022

debsmita1 commented Dec 22, 2021 •

edited

Loading

invincibleJai commented Dec 30, 2021 •

edited

Loading