Add application environment details view #175

jeff-phillips-18 · 2022-08-09T12:27:59Z

Fixes

https://issues.redhat.com/browse/HAC-1943
https://issues.redhat.com/browse/HAC-1944

Description

Updates the application details view to use the application environment cards as drill downs into an application's environment details view.
Add an application environment details view showing components in the application. This view is switchable from a list view to a topology graph view with a side panel showing more details for the selected component.

Type of change

Feature

Screen shots / Gifs for design review

How to test or reproduce?

Navigate to the application details view.
Select the environment card you wish to view details for.
Select a component in the environment to view the side panel details.
Use the graph/list icons to switch between list and graph views.

Browser conformance:

Chrome
Firefox
Safari
Edge

invincibleJai · 2022-08-09T12:47:02Z

/assign @rohitkrai03
/cc @sahil143 @rottencandy

codecov · 2022-08-09T14:51:36Z

Codecov Report

Merging #175 (8c2b676) into main (af8638f) will increase coverage by 0.32%.
The diff coverage is 71.07%.

@@            Coverage Diff             @@
##             main     #175      +/-   ##
==========================================
+ Coverage   66.61%   66.94%   +0.32%     
==========================================
  Files         319      336      +17     
  Lines        6042     6492     +450     
  Branches     1437     1562     +125     
==========================================
+ Hits         4025     4346     +321     
- Misses       1911     2030     +119     
- Partials      106      116      +10

Impacted Files	Coverage Δ
...nents/ApplicationDetailsView/ComponentListView.tsx	`100.00% <ø> (ø)`
...ponents/ApplicationListView/ApplicationListRow.tsx	`100.00% <ø> (ø)`
src/pages/ApplicationDetailsPage.tsx	`0.00% <0.00%> (ø)`
src/pages/ApplicationEnvironmentDetailsPage.tsx	`0.00% <0.00%> (ø)`
src/pages/ApplicationsPage.tsx	`0.00% <ø> (ø)`
...hared/components/action-menu/ActionMenuContent.tsx	`72.72% <ø> (ø)`
...nvironment/topology/actions/contextMenuActions.tsx	`37.50% <37.50%> (ø)`
src/shared/utils/git-utils.tsx	`38.46% <38.46%> (ø)`
.../ApplicationDetailsView/ApplicationDetailsView.tsx	`90.47% <50.00%> (-2.86%)`	⬇️
...onents/Environment/ApplicationEnvironmentCards.tsx	`57.57% <50.00%> (-4.33%)`	⬇️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af8638f...8c2b676. Read the comment docs.

karthikjeeyar

I have tried locally Initial node position is not centered 🤔

karthikjeeyar · 2022-08-10T09:14:45Z

src/components/ApplicationEnvironment/ApplicationEnvironmentDetailsView.tsx

+    </Bullseye>
+  );
+
+  if (applicationError || !applicationLoaded || environmentError || !environmentLoaded) {


Suggested change

if (applicationError || !applicationLoaded || environmentError || !environmentLoaded) {

if (applicationError || !applicationLoaded || environmentError || !environmentLoaded || !componentsLoaded) {

karthikjeeyar · 2022-08-10T10:17:40Z

src/components/ApplicationEnvironment/ApplicationEnvironmentListItem.tsx

+                </FlexItem>
+                <FlexItem>
+                  <Tooltip content="Route">
+                    <ExternalLink href={componentRouteWebURL} text={<ExternalLinkAltIcon />} />


Clicking on this link opens the sidebar.

jeff-phillips-18 · 2022-08-10T12:07:54Z

There are a couple of issues causing the topology positioning error. Fixes are known but not yet available:

rottencandy · 2022-08-10T12:12:36Z

src/pages/ApplicationEnvironmentDetailsPage.tsx

+import imageUrl from '../imgs/getting-started-illustration.svg';
+import { getQueryArgument } from '../shared/utils';
+
+const GETTING_STARTED_MODAL_KEY = 'application-list-getting-started-modal';


Suggested change

const GETTING_STARTED_MODAL_KEY = 'application-list-getting-started-modal';

const GETTING_STARTED_MODAL_KEY = 'environment-details-getting-started-modal';

rottencandy · 2022-08-10T12:12:40Z

src/shared/utils/git-utils.tsx

+  INVALID = '',
+}
+
+export const gitUrlRegex =


We have gitUrlRegex already in src/components/ImportForm/utils/validation-utils.ts

jeff-phillips-18 · 2022-08-11T12:33:09Z

cc @beaumorley @Ranelim @serenamarie125

jeff-phillips-18 · 2022-08-17T12:32:55Z

@karthikjeeyar @rottencandy Could you please take another look?

@beaumorley @Ranelim @serenamarie125 Any comments from UX/PM?

karthikjeeyar · 2022-08-17T12:33:07Z

Hard coded values such as commit info and status & date in environment card will be addressed in separate ticket ?

jeff-phillips-18 · 2022-08-17T12:37:02Z

Hard coded values such as commit info and status & date in environment card will be addressed in separate ticket ?

Yes, unless those are now available. The hard coded values were there already.

rohitkrai03

When switching from list view to graph view with an item selected. I cannot select the node again once the sidebar is closed.

Screen.Recording.2022-08-17.at.7.27.45.PM.mov

Also, should we even show the revision details right now with dummy data or wait for the API to provide information and then add it? cc: @serenamarie125 @beaumorley

rohitkrai03 · 2022-08-17T13:36:51Z

config/remotePlugin.js

+    {
+      type: 'core.page/route',
+      properties: {
+        path: '/app-studio/application-environment-details',


HAC Core now support routes with params. So maybe we can use that instead of query params here -

Suggested change

path: '/app-studio/application-environment-details',

path: '/app-studio/applications/:appName/environments/:envName',

PS: We also need to update the current application route to support route param. Something like - /app-studio/applications/:appName

rohitkrai03 · 2022-08-17T13:39:51Z

src/components/ApplicationDetailsView/ApplicationDetailsView.tsx

        actions={actions}
      >
        <PageSection>
          <Flex>


Do we need this flex anymore?

jeff-phillips-18 · 2022-08-18T18:34:46Z

Collapsed state form environments on Application page:

serenamarie125 · 2022-08-18T18:44:47Z

@rohitkrai03 in my opinion, the more we can show the better. It helps us to get feedback.

itsptk · 2022-08-18T20:46:58Z

I created a doc of notes about the implementation of the application view, and have included the environment view specific notes in this comment, since those seem to be the focus of this PR.

“Application healthy” “Deployment strategy” should all have the second word lowercase
“Automated” should be “Automatic” deployment strategy
“Last deploy” label should be “Last deployment”
Route, Image, and Code repository links in the side panel should all seemingly include external link icons at the end
A kebab menu was shown in the topology side panel in the design to match the list view, whereas an Actions menu appears in the implementation

Note: This list doesn't capture any sort of adjustments based on the absence of the ‘Components’ card and the selection behavior that drives changing the detailed content below. My understanding is that this was an iteration for milestone 7 of this view's development and those interactions could still come in later milestones.

jeff-phillips-18 · 2022-08-18T21:18:59Z

“Application healthy” “Deployment strategy” should all have the second word lowercase

👍

“Automated” should be “Automatic” deployment strategy

This is in the application detail page, I have not updated that and I know there are changes being made. Not part of this PR.

“Last deploy” label should be “Last deployment”

👍

Route, Image, and Code repository links in the side panel should all seemingly include external link icons at the end

It looks bad to me to have 4 links in the side panel each with the external link icon. To me it is fairly obvious that they are links and therefore also expect them to open in a new tab. Seems extraneous and cluttering. If UX feels strongly I will add them, but against my better judgement.

A kebab menu was shown in the topology side panel in the design to match the list view, whereas an Actions menu appears in the implementation

The kebab menu looks bad next to the close (x) button. Openshift uses the Actions dropdown so I went with that. If UX feels strongly I will change to the kebab, but against my better judgement.

jeff-phillips-18 · 2022-08-19T11:19:47Z

itsptk · 2022-08-19T19:38:54Z

Note: This list doesn't capture any sort of adjustments based on the absence of the ‘Components’ card and the selection behavior that drives changing the detailed content below. My understanding is that this was an iteration for milestone 7 of this view's development and those interactions could still come in later milestones.

Regarding this interaction, @jeff-phillips-18 mentioned that he might be able to implement the switching interaction in M7 as a separate PR, just so we could try iterating on the view to see how it looks and where any issues lie. If that PR can't merge in time or we decide not to go that route for now, at least this existing PR will have already merged with the environment view functionality as a drill-in for M7 and we can revisit in M8.

I suspect when we implement the switching behavior, we will confirm that the space for the topology graph view and side panel is fairly constrained when the cards are expanded. My hope is that by iterating on that PR we can find an acceptable layout that offers the user sufficient space to interact with the environment’s components, and also still retains the card flow view of the components into the Development environment into subsequent environments, since I do think that is beneficial to orienting the user, and also very explicitly shows the user what environments’ details they are looking at (which seems to be important.)

cc @beaumorley @rohitkrai03 @serenamarie125

rohitkrai03

@jeff-phillips-18 I am still seeing the issue where I am not able to select the node if i switch from list view to graph view with a node selected. Also the search is not highlighting the nodes.

jeff-phillips-18 · 2022-08-23T20:11:08Z

@rohitkrai03 Every other view filters out the non-matches. Do we want this view to be different? Only the graph view or both the graph and list view?

karthikjeeyar · 2022-08-24T13:55:08Z

/lgtm

jeff-phillips-18 · 2022-08-25T10:44:20Z

@rohitkrai03 Any further thoughts?

karthikjeeyar · 2022-08-25T18:47:36Z

/retest

karthikjeeyar · 2022-08-25T18:47:49Z

/lgtm

karthikjeeyar · 2022-08-26T05:17:13Z

There is a test failure due to new route in hacbs ImportForm submit handler.

openshift-ci · 2022-08-26T11:21:09Z

@jeff-phillips-18: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

karthikjeeyar · 2022-08-26T11:36:58Z

/lgtm

rohitkrai03

/approve

openshift-ci · 2022-08-26T15:31:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jeff-phillips-18, rohitkrai03

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [rohitkrai03]

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

openshift-ci bot requested review from abhinandan13jan and christianvogt August 9, 2022 12:29

openshift-ci bot assigned rohitkrai03 Aug 9, 2022

openshift-ci bot requested review from rottencandy and sahil143 August 9, 2022 12:47

jeff-phillips-18 force-pushed the environment-details branch from f6cf2d1 to 779776a Compare August 9, 2022 14:34

karthikjeeyar reviewed Aug 10, 2022

View reviewed changes

rottencandy reviewed Aug 10, 2022

View reviewed changes

jeff-phillips-18 force-pushed the environment-details branch from ed2a0c1 to 499f9ec Compare August 17, 2022 13:17

rohitkrai03 suggested changes Aug 17, 2022

View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2022

jeff-phillips-18 force-pushed the environment-details branch from 499f9ec to 74dcd3f Compare August 18, 2022 18:36

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2022

jeff-phillips-18 force-pushed the environment-details branch from 74dcd3f to 04e71db Compare August 18, 2022 19:51

rohitkrai03 suggested changes Aug 23, 2022

View reviewed changes

jeff-phillips-18 force-pushed the environment-details branch 2 times, most recently from aacf940 to 22eeeaa Compare August 24, 2022 13:53

openshift-ci bot assigned karthikjeeyar Aug 24, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 24, 2022

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2022

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 25, 2022

jeff-phillips-18 added 6 commits August 25, 2022 12:57

Add application environment details view

2a36aad

Updates per review comments

e3de8d8

Update paths using query arguments, set view options in search params

cf02dbe

Update header text - per UX

25fa72a

Fix for graph view selection, add more tests

9d68e16

Add empty state for environment cards, hide replica count if not set

e290f2a

jeff-phillips-18 force-pushed the environment-details branch from 320a056 to c5e027b Compare August 25, 2022 16:59

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 25, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2022

Fix for navigation from ImportForm to application page

8c2b676

jeff-phillips-18 force-pushed the environment-details branch from c5e027b to 8c2b676 Compare August 26, 2022 11:05

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2022

rohitkrai03 approved these changes Aug 26, 2022

View reviewed changes

rohitkrai03 merged commit 516a64a into openshift:main Aug 26, 2022

rohitkrai03 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 26, 2022

	if (applicationError \|\| !applicationLoaded \|\| environmentError \|\| !environmentLoaded) {
	if (applicationError \|\| !applicationLoaded \|\| environmentError \|\| !environmentLoaded \|\| !componentsLoaded) {

	const GETTING_STARTED_MODAL_KEY = 'application-list-getting-started-modal';
	const GETTING_STARTED_MODAL_KEY = 'environment-details-getting-started-modal';

	path: '/app-studio/application-environment-details',
	path: '/app-studio/applications/:appName/environments/:envName',

Add application environment details view #175

Add application environment details view #175

Uh oh!

Conversation

jeff-phillips-18 commented Aug 9, 2022

Fixes

Description

Type of change

Screen shots / Gifs for design review

How to test or reproduce?

Browser conformance:

Uh oh!

invincibleJai commented Aug 9, 2022

Uh oh!

codecov bot commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

karthikjeeyar left a comment

Choose a reason for hiding this comment

Uh oh!

karthikjeeyar Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

karthikjeeyar Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 commented Aug 10, 2022

Uh oh!

rottencandy Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

rottencandy Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 commented Aug 11, 2022

Uh oh!

jeff-phillips-18 commented Aug 17, 2022

Uh oh!

karthikjeeyar commented Aug 17, 2022

Uh oh!

jeff-phillips-18 commented Aug 17, 2022

Uh oh!

rohitkrai03 left a comment

Choose a reason for hiding this comment

Uh oh!

rohitkrai03 Aug 17, 2022

Choose a reason for hiding this comment

Uh oh!

rohitkrai03 Aug 17, 2022

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 commented Aug 18, 2022

Uh oh!

serenamarie125 commented Aug 18, 2022

Uh oh!

itsptk commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeff-phillips-18 commented Aug 18, 2022

Uh oh!

jeff-phillips-18 commented Aug 19, 2022

Uh oh!

itsptk commented Aug 19, 2022

Uh oh!

rohitkrai03 left a comment

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 commented Aug 23, 2022

Uh oh!

karthikjeeyar commented Aug 24, 2022

Uh oh!

jeff-phillips-18 commented Aug 25, 2022

Uh oh!

karthikjeeyar commented Aug 25, 2022

Uh oh!

karthikjeeyar commented Aug 25, 2022

Uh oh!

karthikjeeyar commented Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

codecov bot commented Aug 9, 2022 •

edited

Loading

itsptk commented Aug 18, 2022 •

edited

Loading

karthikjeeyar commented Aug 26, 2022 •

edited

Loading