Add the component and utility for helm release grouping #3913

sahil143 · 2020-01-09T18:28:11Z

ODC-story: https://issues.redhat.com/browse/ODC-2622

This PR adds the grouping for the helm release nodes

updated screentshot

[TODO]:

Add unit test for the utility
Add Helm icon the release group label
disable drag-drop for helm- release group.

andrewballantyne · 2020-01-09T20:09:18Z

/kind feature

karthikjeeyar · 2020-01-14T12:14:47Z

frontend/packages/dev-console/src/components/topology/components/groups/HelmRelease.scss

+  &__label {
+    cursor: pointer;
+  }
+}


add extra line here

karthikjeeyar · 2020-01-14T12:15:29Z

frontend/public/imgs/logos/helm.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" role="img" viewBox="-6.49 -6.74 316.49 363.74"><mask id="a" fill="#fff"><path fill-rule="evenodd" d="m0 0h313.303155v159.864865h-313.303155z"/></mask><mask id="b" fill="#fff"><path fill-rule="evenodd" d="m0 0h313.303155v159.864865h-313.303155z"/></mask><g fill="none" fill-rule="evenodd"><path fill="#000" d="m11.6785714 189h19.7858357v26.789h23.9037v-26.789h19.7858358v75.25h-19.7858358v-28.695333h-23.9037v28.695333h-19.7858357zm86.1738429 75.25v-75.25h46.8030427v16.354333h-27.017207v12.240667h23.9037v16.655333h-23.9037v13.846h27.017207v16.153667zm68.4971567 0v-75.25h19.785836v55.384h27.117643v19.866zm77.536372-75.25 30.733328 27.892667 30.632893-27.892667h8.938779v75.25h-19.886272v-38.628333l-19.6854 17.959666-19.785835-17.859333v38.528h-19.886272v-75.25z" transform="translate(-11 -51)"/><g><g fill="#000" mask="url(#a)" transform="matrix(1 0 0 -1 .958 404)"><path d="m203.460676 95.6875425c6.93631 0 12.559301-14.8092194 12.559301-33.0773172s-5.622991-33.0773172-12.559301-33.0773172c-6.936311 0-12.559301 14.8092194-12.559301 33.0773172s5.62299 33.0773172 12.559301 33.0773172z" transform="rotate(35 137.931 151.55)"/><path d="m30.1423223 95.6875425c6.9363104 0 12.559301-14.8092194 12.559301-33.0773172s-5.6229906-33.0773172-12.559301-33.0773172-12.5593009 14.8092194-12.5593009 33.0773172 5.6229905 33.0773172 12.5593009 33.0773172z" transform="scale(-1 1) rotate(35 -104.692 -68.258)"/><path d="m116.732815 66.2752676c6.936311 0 12.559301-14.8092193 12.559301-33.0773172 0-18.2680978-5.62299-33.07731713-12.559301-33.07731713-6.93631 0-12.559301 14.80921933-12.559301 33.07731713 0 18.2680979 5.622991 33.0773172 12.559301 33.0773172z" transform="matrix(-1 0 0 1 272.629 53.67)"/></g><path stroke="#000" stroke-width="20" d="m251.467006 173.099849c-20.230076-33.609969-56.889565-56.067908-98.755776-56.067908-40.720798 0-76.5158766 21.245901-97.0586959 53.334588m2.1981107 129.169534c20.8403036 30.232701 55.5559042 50.026591 94.8605852 50.026591 39.376099 0 74.146424-19.865887 94.974049-50.191495" mask="url(#a)" transform="matrix(1 0 0 -1 .958 404)"/></g><g><g fill="#000" mask="url(#b)" transform="translate(.958 -51)"><path d="m203.460676 95.6875425c6.93631 0 12.559301-14.8092194 12.559301-33.0773172s-5.622991-33.0773172-12.559301-33.0773172c-6.936311 0-12.559301 14.8092194-12.559301 33.0773172s5.62299 33.0773172 12.559301 33.0773172z" transform="rotate(35 141.831 150.32)"/><path d="m30.1423223 95.6875425c6.9363104 0 12.559301-14.8092194 12.559301-33.0773172s-5.6229906-33.0773172-12.559301-33.0773172-12.5593009 14.8092194-12.5593009 33.0773172 5.6229905 33.0773172 12.5593009 33.0773172z" transform="scale(-1 1) rotate(35 -100.792 -69.488)"/><path d="m116.732815 66.2752676c6.936311 0 12.559301-14.8092193 12.559301-33.0773172 0-18.2680978-5.62299-33.07731713-12.559301-33.07731713-6.93631 0-12.559301 14.80921933-12.559301 33.07731713 0 18.2680979 5.622991 33.0773172 12.559301 33.0773172z" transform="matrix(-1 0 0 1 272.629 51.211)"/></g><path stroke="#000" stroke-width="20" d="m251.467006 170.64039c-20.230076-33.609969-56.889565-56.067908-98.755776-56.067908-40.720798 0-76.5158766 21.2459-97.0586959 53.334587m2.1981107 129.169534c20.8403036 30.232702 55.5559042 50.026591 94.8605852 50.026591 39.376099 0 74.146424-19.865886 94.974049-50.191494" mask="url(#b)" transform="translate(.958 -51)"/></g></g></svg>


add new line at the end.

karthikjeeyar · 2020-01-14T12:34:11Z

frontend/packages/dev-console/src/components/topology/components/groups/HelmRelease.tsx

+            y={y + height + 20}
+            paddingX={8}
+            paddingY={4}
+            kind="HelmRelease"


Instead of hard coding HelmRelease can we use element.getData().kind? Or is there a model for Helm release that we can use?

There is no model or kind for this group.

karthikjeeyar · 2020-01-14T12:40:17Z

frontend/packages/dev-console/src/components/topology/topology-utils.ts

+          if (isHelmReleaseNode(deploymentConfig)) {
+            groupsData = [
+              ...getTopologyHelmReleaseGroupItem(
+                deploymentConfig,
+                topologyGraphAndNodeData.graph.groups,
+              ),
+            ];
+          } else {
+            groupsData = [
+              ...getTopologyGroupItems(deploymentConfig, topologyGraphAndNodeData.graph.groups),
+            ];
+          }


It would be good to move HelmReleaseGroupItem call into getTopologyGroupItems function, we could return different group based on the conditions there. So that later on we can have all group related login in one place rather that adding more if else here. Any thoughts ?

I agree. fixed

jeff-phillips-18 · 2020-01-16T14:19:23Z

@openshift/team-devconsole-ux For the type badge, should it be located after the label text as shown above more more inline with what is shown at https://docs.google.com/document/d/1TjzqcUgz6A6_3SiUsqCG1qsHK9z6hAKWMqvEYW7a1ew/edit#heading=h.g319y27ad91p (before the resource badge).

sahil143 · 2020-01-16T14:59:18Z

@jeff-phillips-18 there seems to be a different design for the helm group icon. I followed this design which is marked final https://docs.google.com/document/d/1JVDrPC5Ow2Qmxn9mH1BmPuJhukP5op4Kp4z34xZ95qE/edit#heading=h.aaw0sjx7rogf

jeff-phillips-18 · 2020-01-16T15:12:45Z

frontend/packages/dev-console/src/components/topology/componentFactory.ts

@@ -123,6 +149,7 @@ class ComponentFactory {
        case TYPE_REVISION_TRAFFIC:
          return TrafficLink;
        case TYPE_WORKLOAD:
+          console.log('##################33', kind, type);


Please remove

karthikjeeyar · 2020-01-16T15:59:45Z

Labels and icons should be blurred out if it does'nt match the filter

sahil143 · 2020-01-20T05:14:31Z

Labels and icons should be blurred out if it doesn't match the filter

@karthikjeeyar fixed!
Updated screendshot

karthikjeeyar · 2020-01-21T03:39:20Z

@sahil143 As per the final design doc the icon has to be in the front.

Note: if the helm chart doesn't have icon then default it to helm icon

cc: @serenamarie125 @openshift/team-devconsole-ux

christianvogt · 2020-01-21T05:31:26Z

frontend/packages/dev-console/src/components/topology/topology-utils.ts

+  const releaseExists = _.some(groups, { name: releaseLabel });
+  if (!releaseExists) {
+    groups.push({
+      id: `group:${releaseLabel}`,


Your prefix would be better to be helm specific. Perhaps helm-release:

christianvogt · 2020-01-21T05:34:35Z

frontend/packages/dev-console/src/components/svg/SvgBoxedText.tsx

+  const [badgeSize, badgeRef] = useSize([kind]);
+  const [labelSize, labelRef] = useSize([children, textSize, badgeSize]);
+  const iconSpace = kind && badgeSize ? badgeSize.width + paddingX : 0;
+  const labelSizeWidth = icon ? paddingX * 2 + iconSpace + iconSize / 2 : paddingX * 2 + iconSpace;


Remove the duplicate parts to before the ternary operation.

christianvogt · 2020-01-21T05:37:02Z

frontend/packages/dev-console/src/components/svg/SvgBoxedText.tsx

+          <image
+            x={x + labelSize.width / 2 + paddingX - iconSize / 2 + iconPadding}
+            y={y + iconPadding}
+            width={30}


Iconsize - iconpadding * 2?

You seem to be doing a lot computed values but this size remains static even though it is as relative value.

christianvogt · 2020-01-21T05:41:33Z

frontend/packages/dev-console/src/components/topology/components/groups/HelmRelease.tsx

+const HelmRelease: React.FC<HelmReleaseProps> = ({ element, dragging, filters }) => {
+  const [hover, hoverRef] = useHover();
+  const { x, y, width, height } = element.getBounds();
+  const dragNodeRef = useDragNode()[1];


Use deconstructing

christianvogt · 2020-01-21T05:49:06Z

frontend/packages/dev-console/src/components/svg/SvgBoxedText.tsx

  // TODO remove with 2.0
  onMouseEnter?: React.MouseEventHandler<SVGGElement>;
  onMouseLeave?: React.MouseEventHandler<SVGGElement>;
 }

 const FILTER_ID = 'SvgBoxedTextDropShadowFilterId';
+const iconFilterID = 'SVGBoxedTextRectIconFilter';
+const iconSize = 36;


This is a computed value of 30 + padding * 2

christianvogt · 2020-01-21T05:50:39Z

frontend/packages/dev-console/src/components/topology/topology-utils.ts

+export const isHelmReleaseNode = (obj: K8sResourceKind): boolean => {
+  return (
+    _.get(obj, ['metadata', 'labels', 'heritage'], null) === 'Helm' ||
+    !!_.get(obj, ['metadata', 'labels', 'chart'], false)


Use optional chaining

sahil143 · 2020-01-21T13:32:06Z

@karthikjeeyar @christianvogt Since the UX is changed. I have raised a bug to fix the location of the icon the grouping label because same is taken care by @jeff-phillips-18 PR: #3981. Once that PR is merged, We can fix this in a bug.
Any thoughts?

initial implementation initial implementation remove styles for selected and fine tune initial implementation align the item to UX designs add tests for utility and update test use fill attribute istead of styles update snapshot add corner radius to helm icon refactor disable dnd for helm release node decrease opacity on filtered items fix llint error lint and destructuring

andrewballantyne · 2020-01-21T15:30:07Z

@karthikjeeyar @christianvogt Since the UX is changed. I have raised a bug to fix the location of the icon the grouping label because same is taken care by @jeff-phillips-18 PR: #3981. Once that PR is merged, We can fix this in a bug.
Any thoughts?

Not really following this conversation - but we can get core functionality in now and shore up the UX differences through the bug afterwards.

karthikjeeyar · 2020-01-21T18:53:22Z

Verified locally
/lgtm

christianvogt · 2020-01-22T02:37:32Z

/approve

serenamarie125

This looks good, thanks for updating the location of the helm icon near the app label. That change was made for topology groupings after the helm chart/release designs were set. Thanks for updating!

openshift-ci-robot · 2020-01-22T02:44:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, karthikjeeyar, sahil143, serenamarie125

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/OWNERS~~ [christianvogt]

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

openshift-ci-robot requested review from nemesis09 and vikram-raj January 9, 2020 18:29

openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 9, 2020

sahil143 force-pushed the helm-grouping branch from b3213b0 to 4dc667a Compare January 14, 2020 06:18

openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. component/core Related to console core functionality labels Jan 14, 2020

sahil143 force-pushed the helm-grouping branch from 4dc667a to 42438de Compare January 14, 2020 06:28

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 14, 2020

sahil143 changed the title ~~[WIP] Add the component and utility for helm release grouping~~ Add the component and utility for helm release grouping Jan 14, 2020

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 14, 2020

sahil143 force-pushed the helm-grouping branch from 42438de to dfa2e4d Compare January 14, 2020 06:34

karthikjeeyar reviewed Jan 14, 2020

View reviewed changes

sahil143 force-pushed the helm-grouping branch 2 times, most recently from 834ab91 to 90e1e79 Compare January 16, 2020 05:03

jeff-phillips-18 reviewed Jan 16, 2020

View reviewed changes

sahil143 force-pushed the helm-grouping branch from 90e1e79 to 6c0fe6b Compare January 20, 2020 05:07

sahil143 force-pushed the helm-grouping branch from 6c0fe6b to f189115 Compare January 20, 2020 08:35

rohitkrai03 mentioned this pull request Jan 20, 2020

Add support for listing and installation of Helm Charts from Dev Catalog #3850

Merged

christianvogt reviewed Jan 21, 2020

View reviewed changes

sahil143 force-pushed the helm-grouping branch from f189115 to a63c5ba Compare January 21, 2020 14:06

openshift-ci-robot assigned karthikjeeyar Jan 21, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 21, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 2020

serenamarie125 approved these changes Jan 22, 2020

View reviewed changes

openshift-merge-robot merged commit 0e8ff95 into openshift:master Jan 22, 2020

spadgett added this to the v4.4 milestone Jan 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the component and utility for helm release grouping #3913

Add the component and utility for helm release grouping #3913

sahil143 commented Jan 9, 2020 •

edited

andrewballantyne commented Jan 9, 2020

karthikjeeyar Jan 14, 2020

sahil143 Jan 16, 2020

karthikjeeyar Jan 14, 2020

sahil143 Jan 16, 2020

karthikjeeyar Jan 14, 2020

sahil143 Jan 15, 2020

karthikjeeyar Jan 14, 2020

sahil143 Jan 16, 2020 •

edited

jeff-phillips-18 commented Jan 16, 2020

sahil143 commented Jan 16, 2020

jeff-phillips-18 Jan 16, 2020

sahil143 Jan 20, 2020

karthikjeeyar commented Jan 16, 2020

sahil143 commented Jan 20, 2020

karthikjeeyar commented Jan 21, 2020

christianvogt Jan 21, 2020

christianvogt Jan 21, 2020

christianvogt Jan 21, 2020 •

edited

christianvogt Jan 21, 2020

christianvogt Jan 21, 2020

christianvogt Jan 21, 2020

sahil143 commented Jan 21, 2020

andrewballantyne commented Jan 21, 2020

karthikjeeyar commented Jan 21, 2020

christianvogt commented Jan 22, 2020

serenamarie125 left a comment

openshift-ci-robot commented Jan 22, 2020

		@@ -0,0 +1 @@
		<svg xmlns="http://www.w3.org/2000/svg" role="img" viewBox="-6.49 -6.74 316.49 363.74"><mask id="a" fill="#fff"><path fill-rule="evenodd" d="m0 0h313.303155v159.864865h-313.303155z"/></mask><mask id="b" fill="#fff"><path fill-rule="evenodd" d="m0 0h313.303155v159.864865h-313.303155z"/></mask><g fill="none" fill-rule="evenodd"><path fill="#000" d="m11.6785714 189h19.7858357v26.789h23.9037v-26.789h19.7858358v75.25h-19.7858358v-28.695333h-23.9037v28.695333h-19.7858357zm86.1738429 75.25v-75.25h46.8030427v16.354333h-27.017207v12.240667h23.9037v16.655333h-23.9037v13.846h27.017207v16.153667zm68.4971567 0v-75.25h19.785836v55.384h27.117643v19.866zm77.536372-75.25 30.733328 27.892667 30.632893-27.892667h8.938779v75.25h-19.886272v-38.628333l-19.6854 17.959666-19.785835-17.859333v38.528h-19.886272v-75.25z" transform="translate(-11 -51)"/><g><g fill="#000" mask="url(#a)" transform="matrix(1 0 0 -1 .958 404)"><path d="m203.460676 95.6875425c6.93631 0 12.559301-14.8092194 12.559301-33.0773172s-5.622991-33.0773172-12.559301-33.0773172c-6.936311 0-12.559301 14.8092194-12.559301 33.0773172s5.62299 33.0773172 12.559301 33.0773172z" transform="rotate(35 137.931 151.55)"/><path d="m30.1423223 95.6875425c6.9363104 0 12.559301-14.8092194 12.559301-33.0773172s-5.6229906-33.0773172-12.559301-33.0773172-12.5593009 14.8092194-12.5593009 33.0773172 5.6229905 33.0773172 12.5593009 33.0773172z" transform="scale(-1 1) rotate(35 -104.692 -68.258)"/><path d="m116.732815 66.2752676c6.936311 0 12.559301-14.8092193 12.559301-33.0773172 0-18.2680978-5.62299-33.07731713-12.559301-33.07731713-6.93631 0-12.559301 14.80921933-12.559301 33.07731713 0 18.2680979 5.622991 33.0773172 12.559301 33.0773172z" transform="matrix(-1 0 0 1 272.629 53.67)"/></g><path stroke="#000" stroke-width="20" d="m251.467006 173.099849c-20.230076-33.609969-56.889565-56.067908-98.755776-56.067908-40.720798 0-76.5158766 21.245901-97.0586959 53.334588m2.1981107 129.169534c20.8403036 30.232701 55.5559042 50.026591 94.8605852 50.026591 39.376099 0 74.146424-19.865887 94.974049-50.191495" mask="url(#a)" transform="matrix(1 0 0 -1 .958 404)"/></g><g><g fill="#000" mask="url(#b)" transform="translate(.958 -51)"><path d="m203.460676 95.6875425c6.93631 0 12.559301-14.8092194 12.559301-33.0773172s-5.622991-33.0773172-12.559301-33.0773172c-6.936311 0-12.559301 14.8092194-12.559301 33.0773172s5.62299 33.0773172 12.559301 33.0773172z" transform="rotate(35 141.831 150.32)"/><path d="m30.1423223 95.6875425c6.9363104 0 12.559301-14.8092194 12.559301-33.0773172s-5.6229906-33.0773172-12.559301-33.0773172-12.5593009 14.8092194-12.5593009 33.0773172 5.6229905 33.0773172 12.5593009 33.0773172z" transform="scale(-1 1) rotate(35 -100.792 -69.488)"/><path d="m116.732815 66.2752676c6.936311 0 12.559301-14.8092193 12.559301-33.0773172 0-18.2680978-5.62299-33.07731713-12.559301-33.07731713-6.93631 0-12.559301 14.80921933-12.559301 33.07731713 0 18.2680979 5.622991 33.0773172 12.559301 33.0773172z" transform="matrix(-1 0 0 1 272.629 51.211)"/></g><path stroke="#000" stroke-width="20" d="m251.467006 170.64039c-20.230076-33.609969-56.889565-56.067908-98.755776-56.067908-40.720798 0-76.5158766 21.2459-97.0586959 53.334587m2.1981107 129.169534c20.8403036 30.232702 55.5559042 50.026591 94.8605852 50.026591 39.376099 0 74.146424-19.865886 94.974049-50.191494" mask="url(#b)" transform="translate(.958 -51)"/></g></g></svg>

Add the component and utility for helm release grouping #3913

Add the component and utility for helm release grouping #3913

Conversation

sahil143 commented Jan 9, 2020 • edited

andrewballantyne commented Jan 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sahil143 Jan 16, 2020 • edited

Choose a reason for hiding this comment

jeff-phillips-18 commented Jan 16, 2020

sahil143 commented Jan 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karthikjeeyar commented Jan 16, 2020

sahil143 commented Jan 20, 2020

karthikjeeyar commented Jan 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christianvogt Jan 21, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sahil143 commented Jan 21, 2020

andrewballantyne commented Jan 21, 2020

karthikjeeyar commented Jan 21, 2020

christianvogt commented Jan 22, 2020

serenamarie125 left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jan 22, 2020

sahil143 commented Jan 9, 2020 •

edited

sahil143 Jan 16, 2020 •

edited

christianvogt Jan 21, 2020 •

edited