Bug 1798177: Add expand/collapse of operator groups, type icons #4190

jeff-phillips-18 · 2020-02-04T18:33:46Z

Fixes:
https://issues.redhat.com/browse/ODC-2851
https://issues.redhat.com/browse/ODC-2894

Analysis / Root cause:
The expand/collapse groupings never took the Operator Groups into consideration so they were not expanded/collapsed when the filter was toggled.

Solution Description:
Add the necessary handling of the filter for the expand/collapse of operator groups. Allow operator groups to be selected, though there is no side panel for them at this time. To better distinguish the collapsed group types, add the grouping type icon to the upper left corner of collapsed groups.

Browser conformance:

Chrome
Firefox
Safari
Edge

Sample Screenshots:

cc @openshift/team-devconsole-ux

openshift-ci-robot · 2020-02-04T18:33:52Z

@jeff-phillips-18: This pull request references Bugzilla bug 1798177, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1798177: Add expand/collapse of operator groups, type icons

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.

andrewballantyne · 2020-02-04T19:46:08Z

/kind bug

serenamarie125 · 2020-02-05T02:21:00Z

FYI @Veethika

serenamarie125

Knative grouping border should be the same as other groupings (Operator backed groupings & helm groupings). Unsure if that's part of this bug or not, but providing the feedback.

Otherwise, the expand/collapse & associated type icons look good

karthikjeeyar · 2020-02-05T12:07:20Z

...packages/dev-console/src/components/topology/components/nodes/OperatorBackedServiceGroup.tsx

+      ref={hoverRef}
+      onClick={onSelect}
+      className={classNames('odc-operator-backed-service', {
+        'is-selected': selected,


On selecting expanded group, it is not adding the highlight which is added when the group is collapsed.

You actually don't need this line because it is only needed on the __bg node.

jeff-phillips-18 · 2020-02-05T19:25:27Z

@serenamarie125 Updated to use the same styling for Helm, Knative, and Operator groups.

abhi-kn · 2020-02-05T20:01:22Z

@serenamarie125
A small observation not directly related to this PR. If an application group has Knative service & on collapsing application group, the collapsed app group is expected to display resource name as 'Knative Services' with count as per design. But since we use resource kind as label to display and Knative resources have kind value as Service, label displayed is 'Service' instead of 'Knative Service'. Is this acceptable or do we need to display the label as 'Knative Service'?

Actual result:

Expected result:

cc: @jeff-phillips-18

christianvogt · 2020-02-06T02:26:29Z

...packages/dev-console/src/components/topology/components/nodes/OperatorBackedServiceGroup.tsx

+      ref={hoverRef}
+      onClick={onSelect}
+      className={classNames('odc-operator-backed-service', {
+        'is-selected': selected,


You actually don't need this line because it is only needed on the __bg node.

christianvogt · 2020-02-06T02:31:48Z

...packages/dev-console/src/components/topology/components/nodes/OperatorBackedServiceGroup.tsx

+        <SvgBoxedText
+          className="odc-base-node__label"
+          x={x + width / 2}
+          y={y + height + 30}


Maybe we should update this to 24px now because the labels are overlapping the app group border. Or we add more padding to the app groups.

christianvogt · 2020-02-06T02:34:02Z

@abhi-kn I think it's worth raising an issue for discussion around the naming and not for this PR.
Because Service is the name of the resource. We'd have to make the change console wide or specifically a one off in topology. That being said we also don't name other knative resources Knative Revision or Knative Route.
cc @openshift/team-devconsole-ux

jeff-phillips-18 · 2020-02-06T16:25:25Z

/test analyze

christianvogt · 2020-02-06T22:01:30Z

/lgtm
/approve

openshift-bot · 2020-02-07T00:12:58Z

/retest

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

openshift-bot · 2020-02-07T00:24:53Z

/retest

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

serenamarie125 · 2020-02-07T01:06:41Z

A small observation not directly related to this PR. If an application group has Knative service & on collapsing application group, the collapsed app group is expected to display resource name as 'Knative Services' with count as per design. But since we use resource kind as label to display and Knative resources have kind value as Service, label displayed is 'Service' instead of 'Knative Service'. Is this acceptable or do we need to display the label as 'Knative Service'?

@abhi-kn I think this is a problem, IMO it should be KSVC if the Application Grouping has a Knative Service inside of it. Interesting, as I found something likely connected which I mentioned on the tmp-odc-topology channel today.

serenamarie125

Jeff this looks great! I see both the operator backed grouping and Knative service groupings here, and noticed that the knative logo has also been removed from the revisions 👍
Approved from a UX perspective ( I know that some additional visual changes will happen based on Veethika's subsequent visual specs.

openshift-ci-robot · 2020-02-07T01:10:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, jeff-phillips-18, 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/packages/dev-console/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 · 2020-02-07T01:41:03Z

@jeff-phillips-18: All pull requests linked via external trackers have merged. Bugzilla bug 1798177 has been moved to the MODIFIED state.

In response to this:

Bug 1798177: Add expand/collapse of operator groups, type icons

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.

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Feb 4, 2020

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. component/dev-console Related to dev-console labels Feb 4, 2020

openshift-ci-robot requested review from abhi-kn and rohitkrai03 February 4, 2020 18:34

openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Feb 4, 2020

serenamarie125 reviewed Feb 5, 2020

View reviewed changes

karthikjeeyar reviewed Feb 5, 2020

View reviewed changes

christianvogt reviewed Feb 6, 2020

View reviewed changes

jeff-phillips-18 added 4 commits February 6, 2020 07:31

Bug 1798177: Add expand/collapse of operator groups, type icons

1ace9be

Fix for operator group selection

3515ba5

Make group backgrounds consistent

e5f373a

Updates per review comments

20ce393

jeff-phillips-18 force-pushed the collapse-groups branch from 6472e60 to 20ce393 Compare February 6, 2020 12:50

openshift-ci-robot assigned christianvogt Feb 6, 2020

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 6, 2020

serenamarie125 approved these changes Feb 7, 2020

View reviewed changes

openshift-merge-robot merged commit 32a85f9 into openshift:master Feb 7, 2020

spadgett added this to the v4.4 milestone Feb 17, 2020

jeff-phillips-18 deleted the collapse-groups branch December 2, 2020 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1798177: Add expand/collapse of operator groups, type icons #4190

Bug 1798177: Add expand/collapse of operator groups, type icons #4190

jeff-phillips-18 commented Feb 4, 2020

openshift-ci-robot commented Feb 4, 2020

andrewballantyne commented Feb 4, 2020

serenamarie125 commented Feb 5, 2020

serenamarie125 left a comment

karthikjeeyar Feb 5, 2020

jeff-phillips-18 Feb 5, 2020

christianvogt Feb 6, 2020

jeff-phillips-18 commented Feb 5, 2020

abhi-kn commented Feb 5, 2020 •

edited

christianvogt Feb 6, 2020

christianvogt Feb 6, 2020

christianvogt commented Feb 6, 2020

jeff-phillips-18 commented Feb 6, 2020

christianvogt commented Feb 6, 2020

openshift-bot commented Feb 7, 2020

openshift-bot commented Feb 7, 2020

serenamarie125 commented Feb 7, 2020

serenamarie125 left a comment

openshift-ci-robot commented Feb 7, 2020

openshift-ci-robot commented Feb 7, 2020

Bug 1798177: Add expand/collapse of operator groups, type icons #4190

Bug 1798177: Add expand/collapse of operator groups, type icons #4190

Conversation

jeff-phillips-18 commented Feb 4, 2020

openshift-ci-robot commented Feb 4, 2020

andrewballantyne commented Feb 4, 2020

serenamarie125 commented Feb 5, 2020

serenamarie125 left a comment

Choose a reason for hiding this comment

karthikjeeyar Feb 5, 2020

Choose a reason for hiding this comment

jeff-phillips-18 Feb 5, 2020

Choose a reason for hiding this comment

christianvogt Feb 6, 2020

Choose a reason for hiding this comment

jeff-phillips-18 commented Feb 5, 2020

abhi-kn commented Feb 5, 2020 • edited

christianvogt Feb 6, 2020

Choose a reason for hiding this comment

christianvogt Feb 6, 2020

Choose a reason for hiding this comment

christianvogt commented Feb 6, 2020

jeff-phillips-18 commented Feb 6, 2020

christianvogt commented Feb 6, 2020

openshift-bot commented Feb 7, 2020

openshift-bot commented Feb 7, 2020

serenamarie125 commented Feb 7, 2020

serenamarie125 left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Feb 7, 2020

openshift-ci-robot commented Feb 7, 2020

abhi-kn commented Feb 5, 2020 •

edited