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.
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
Add Topology sidebar for Operator Backed Services #6017
Add Topology sidebar for Operator Backed Services #6017
Conversation
frontend/packages/dev-console/src/components/topology/operators/TopologyOperatorBackedPanel.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/dev-console/src/components/topology/operators/TopologyOperatorBackedPanel.tsx
Outdated
Show resolved
Hide resolved
<div className="overview__sidebar-pane-head resource-overview__heading"> | ||
<h1 className="co-m-pane__heading"> | ||
<div className="co-m-pane__name co-resource-item"> | ||
<ResourceIcon kind="Operator" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can use model to get kind or pick from obj itself, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a separate icon because ResourceLink
was showing a CSV
icon, but design docs had O
, and O
is also consistent with topology node. I can change it if it's not required.
...d/packages/dev-console/src/components/topology/operators/TopologyOperatorBackedResources.tsx
Outdated
Show resolved
Hide resolved
...d/packages/dev-console/src/components/topology/operators/TopologyOperatorBackedResources.tsx
Outdated
Show resolved
Hide resolved
...d/packages/dev-console/src/components/topology/operators/TopologyOperatorBackedResources.tsx
Outdated
Show resolved
Hide resolved
1ddc9ea
to
f37d945
Compare
Rebased and updated with |
<TopologyHelmReleaseResourcesPanel | ||
manifestResources={finalRes} | ||
releaseNamespace={item.resources.obj.metadata.namespace} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to rename this component to something generic that both operator and helm side panels use. Something like TopologyWorkloadResourcesPanel
. Props like manifestResources
and releaseNamespace
doesn't make sense here. WDYT @jeff-phillips-18 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed.
Looks good @rohitkrai03 @rottencandy One small comment. In the second screen capture with the resources tab selected, the alignment of the resources is off. The "Deployments" title should align with the "D" in the details tab. The deployments should move over the same amount of space (but stay indented from the title like they are now). This looks like it may be resolved in the third screen grab taken by Jai but want to make sure. Thank you! |
f37d945
to
a3f41ca
Compare
The extra space was occuring due to duplicate I also renamed |
</div>, | ||
); | ||
} | ||
return lists; | ||
}, []); | ||
|
||
return <div className="overview__sidebar-pane-body">{resourceLists}</div>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a3f41ca
to
1e044eb
Compare
/lgtm |
/assign |
|
||
const ResourcesSection = () => <TopologyOperatorBackedResources item={item} />; | ||
const DetailsSection = () => ( | ||
<div className="co-m-pane__body"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will align the resource details better with heading and nav items of the sidebar. There's a difference of 10px
between sidebar heading and resource details.
<div className="co-m-pane__body"> | |
<div className="overview__sidebar-pane-body"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
1e044eb
to
7ce03d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: debsmita1, rohitkrai03, rottencandy 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 |
Fixes:
https://issues.redhat.com/browse/ODC-4024
Analysis / Root cause:
As a user, I want to see more details about operator backed services in Topology.
Solution Description:
Add a sidebar that can be opened by clicking an operator backed service from the topology.
Screen shots / Gifs for design review(updated):
cc: @openshift/team-devconsole-ux @rohitkrai03
Browser conformance: