update empty state secondary masthead flows #1836

karthikjeeyar · 2019-06-26T23:03:02Z

This PR contains the following changes

Added the project list as the landing page in Add, Topology, Build and Pipeline pages
Added the create project link in Add page
on clicking a project in the list will change the context on the project selector

Story: https://jira.coreos.com/browse/ODC-1023

sahil143

@karthikjeeyar getting this warning whenever projects list page is refreshed

Warning: Failed prop type: The prop `aria-label` is marked as required in `VirtualGrid`, but its value is `null`.
    in VirtualGrid (created by VirtualTableBody)
    in VirtualTableBody (created by VirtualBody)
    in VirtualBody (created by AutoSizer)
    in div (created by AutoSizer)

frontend/public/components/namespace.jsx

sahil143 · 2019-06-27T11:06:34Z

@karthikjeeyar while clicking on a project from the add page takes me to add page of that project

but creating a new project from the add page redirects to the overview of the newly created Project instead of empty state of add page. Is this the correct flow?

frontend/packages/dev-console/src/components/HintBlock.tsx

karthikjeeyar · 2019-06-27T12:31:06Z

@karthikjeeyar while clicking on a project from the add page takes me to add page of that project

but creating a new project from the add page redirects to the overview of the newly created Project instead of empty state of add page. Is this the correct flow?

@sahil143 Added the support for redirection in the previous PR #1811
once this PR goes in then we will be redirected as intended

karthikjeeyar · 2019-06-27T14:08:14Z

@karthikjeeyar getting this warning whenever projects list page is refreshed

Warning: Failed prop type: The prop `aria-label` is marked as required in `VirtualGrid`, but its value is `null`.
    in VirtualGrid (created by VirtualTableBody)
    in VirtualTableBody (created by VirtualBody)
    in VirtualBody (created by AutoSizer)
    in div (created by AutoSizer)

@sahil143 I have noticed this warning without my changes as well, It is an already existing warning message. we need to fix it in a separate PR
CC: @christianvogt

karthikjeeyar · 2019-06-27T14:18:42Z

@sahil143 added the requested changes.

serenamarie125 · 2019-06-28T01:24:06Z

Just verifying that this new behavior should only occur when "All Projects" is selected in the Project Selector. Looks that way, but I can't verify.

but creating a new project from the add page redirects to the overview of the newly created Project instead of empty state of add page. Is this the correct flow?

This isn't the correct flow. If the user creates a new project from the +Add page, the flow should be:

create a new project
select that project in the project select
remain on the +Add page

christianvogt · 2019-06-28T05:27:37Z

frontend/public/components/namespace.jsx

+};
+ProjectSelectorRow.displayName = 'ProjectSelectorRow';
+
+export const ProjectsTable = props => <Table {...props} aria-label="Projects" Header={ProjectTableHeader} Row={ProjectSelectorRow} virtualize />;


I feel that this should live in dev console for now.

christianvogt · 2019-06-28T05:30:41Z

frontend/packages/dev-console/src/components/HintBlock.tsx

+  title: string;
+  children?: React.ReactNode;
+}
+const HintBlock: React.FC<HintBlockProps> = (props) => (


This component is used in a single location: DefaultPage.
Could you just merge the contents of this into DefaultPage? It doesn't really add much value over the PageHeading and SectionDivider separately.

frontend/packages/dev-console/src/components/AddPage.tsx

+        {namespace ? (
+          <ODCEmptyState title="+Add" />
+        ) : (
+          <DefaultPage title="Add">


christianvogt · 2019-06-28T05:34:19Z

frontend/packages/dev-console/src/plugin.tsx

+      exact: true,
+      path: ['/buildconfigs/all-namespaces', '/buildconfigs/ns/:ns'],
+      loader: async () =>
+        (await import('./components/BuildconfigPage' /* webpackChunkName: "dev-console-import" */))


Update the chunk name.

christianvogt · 2019-06-28T05:40:37Z

frontend/packages/dev-console/src/components/AddPage.tsx

+              Select a project to start adding to it or
+              <button
+                type="button"
+                className="btn btn-link btn-link--no-padding"


btn-link--no-padding doesn't seem to do anything.
The button alignment looks off:

btn-link--no-padding doesn't seem to do anything.
The button alignment looks off:

We use btn-link--no-btn-default-values (although I'd like to rename it).

Looks like we have another btn-link--no-padding we should fix:

packages/dev-console/src/components/source-to-image/SourceToImage.tsx
324: className="btn btn-link btn-link--no-padding"

used btn-link--no-btn-default-values to fix the alignment issue.

christianvogt · 2019-06-28T05:43:48Z

frontend/packages/dev-console/src/components/HintBlock.tsx

+const HintBlock: React.FC<HintBlockProps> = (props) => (
+  <React.Fragment>
+    <PageHeading title={props.title}>{props.children}</PageHeading>
+    <SectionDivider />


This is a very harsh line:

UX design had a soft line:

updated the border color

spadgett · 2019-06-28T18:09:37Z

frontend/packages/dev-console/src/components/DefaultPage.tsx

+      style={{
+        marginTop: '30px',
+        marginBottom: '30px',
+        borderBottom: '0.8px solid var(--pf-global--BackgroundColor--light-300)',


Can we consider $color-grey-background-border(#ccc) for consistency with other pages? We should eventually align with PF4 palette, but we use #ccc elsewhere today.

updated to #ccc

karthikjeeyar · 2019-06-28T19:34:36Z

This isn't the correct flow. If the user creates a new project from the +Add page, the flow should be:

create a new project

select that project in the project select

remain on the +Add page

@serenamarie125 @sahil143 The above-mentioned flow is implemented in the previous PR https://github.com/openshift/console/pull/1811. This PR contains only the project list as a default page when all projects is selected in project selector.

karthikjeeyar · 2019-07-02T06:53:48Z

/retest

christianvogt · 2019-07-02T18:45:05Z

frontend/packages/dev-console/src/components/AddPage.tsx

+        {namespace ? (
+          <ODCEmptyState title="+Add" />
+        ) : (
+          <DefaultPage title="+Add">


@serenamarie125 Please confirm if the page header should be +Add or Add

Page header should just be “Add”

rohitkrai03 · 2019-07-03T08:20:33Z

/lgtm

christianvogt · 2019-07-03T12:22:46Z

frontend/packages/dev-console/src/components/AddPage.tsx

+          <ODCEmptyState title="+Add" />
+        ) : (
+          <DefaultPage title="+Add">
+            <div>


Can you remove this extra div?

christianvogt · 2019-07-03T12:23:04Z

frontend/packages/dev-console/src/components/BuildconfigPage.tsx

+        <BuildsPage {...props} />
+      ) : (
+        <DefaultPage title="Builds">
+          <div>Select a project to view the builds</div>


Can you remove this extra div?

christianvogt · 2019-07-03T12:24:31Z

frontend/packages/dev-console/src/components/pipelines/PipelinesPage.tsx

+    />
+  ) : (
+    <DefaultPage title="Pipelines">
+      <div>Select a project to view the list of pipelines</div>


Can you remove this extra div?

christianvogt · 2019-07-03T12:24:47Z

frontend/packages/dev-console/src/components/topology/TopologyPage.tsx

@@ -67,7 +68,9 @@ const TopologyPage: React.FC<Props> = ({ match, activeApplication }) => {
            render={renderTopology}
          />
        ) : (
-          <EmptyMsg />
+          <DefaultPage title="Topology">
+            <div>Select a project to view the topology</div>


Can you remove this extra div?

christianvogt · 2019-07-03T12:33:25Z

frontend/packages/dev-console/src/components/DefaultPage.tsx

+import { PageHeading } from '@console/internal/components/utils';
+import './DefaultPage.scss';
+
+interface DefaultPageProps {


Export the prop types.

christianvogt · 2019-07-03T12:40:14Z

frontend/public/components/namespace.jsx

+)(({ project, setActiveNamespace }) => (
+  <span className="co-resource-item co-resource-item--truncate">
+    <ResourceIcon kind="Project" />
+    <button


Should add e title to be consistent.

ResourceIcon doesn't have title, only ResourceLink component has the title property

But this button doesn't have the title.

added title prop to the button

christianvogt · 2019-07-03T12:41:06Z

frontend/public/components/namespace.jsx

@@ -136,14 +136,36 @@ const ProjectTableHeader = () => {
 };
 ProjectTableHeader.displayName = 'ProjectTableHeader';

-const ProjectTableRow = ({obj: project, index, key, style}) => {
+const projectLink = connect(


Component names should be Pascal case

karthikjeeyar · 2019-07-03T18:40:00Z

frontend/public/components/namespace.jsx

-        <span className="co-resource-item">
-          <ResourceLink kind="Project" name={project.metadata.name} title={project.metadata.uid} />
-        </span>
+        {customData && ProjectLinkComponent ? (


In order to reuse the ProjectTableRow component in the dev console pages, I have used the customData prop to pass the custom Component to the first cell in the row. @spadgett what do you think about this approach?
cc: @christianvogt

There are couple of ways this could be done. Not sure if one way is really better than the other considering this is sort of an internal implementation detail.

karthikjeeyar · 2019-07-03T18:42:29Z

frontend/public/components/namespace.jsx

-        <ResourceKebab actions={projectMenuActions} kind="Project" resource={project} />
-      </TableData>
+      {
+        actionsEnabled && (


used customData prop here to disable the resource action column.

christianvogt · 2019-07-03T19:26:27Z

/lgtm
/approve

christianvogt · 2019-07-03T20:25:09Z

/hold

christianvogt · 2019-07-04T16:33:58Z

On hold because there are some doubts regarding the customization of routes.

christianvogt · 2019-07-04T16:34:10Z

/approve cancel
/lgtm cancel

karthikjeeyar · 2019-07-09T13:47:52Z

PR has been updated with the additional changes which contain perspective property in page/route extension and overriding default k8s routes to show a different component in dev perspective. This PR is now ready for review.
cc: @christianvogt

frontend/packages/dev-console/src/components/BuildconfigPage.tsx

frontend/packages/dev-console/src/components/AddPage.tsx

frontend/packages/dev-console/src/components/BuildconfigPage.tsx

karthikjeeyar · 2019-07-10T10:14:58Z

/retest

christianvogt · 2019-07-10T13:39:45Z

frontend/packages/dev-console/src/components/BuildconfigPage.tsx

+  );
+};
+
+export default BuildConfigPage;


Rename file to BuildConfigPage.tsx

christianvogt · 2019-07-10T14:33:04Z

frontend/packages/dev-console/src/components/BuildconfigPage.tsx

+        <title>Builds</title>
+      </Helmet>
+      {namespace ? (
+        <BuildConfigsPage filterLabel="Resources by name" {...props} />


Do not change the filterLabel. Make it an optional prop in build-config.tsx

christianvogt · 2019-07-10T14:34:10Z

frontend/packages/dev-console/src/components/BuildconfigPage.tsx

+      {namespace ? (
+        <BuildConfigsPage filterLabel="Resources by name" {...props} />
+      ) : (
+        <DefaultPage title="Build Configs">Select a project to view the build configs</DefaultPage>


The way this message is written differs from pipelines as it adds here list of
Select a project to view the list of pipelines

karthikjeeyar · 2019-07-10T17:08:35Z

/retest

christianvogt · 2019-07-10T19:50:51Z

/hold cancel

christianvogt · 2019-07-10T19:51:01Z

/lgtm
/approve

openshift-ci-robot · 2019-07-10T19:51:12Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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 added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 26, 2019

openshift-ci-robot requested review from jhadvig and sahil143 June 26, 2019 23:03

spadgett added this to the v4.2 milestone Jun 27, 2019

karthikjeeyar force-pushed the landing-page branch 2 times, most recently from a1f00d5 to 4f4bb72 Compare June 27, 2019 09:28

sahil143 reviewed Jun 27, 2019

View reviewed changes

frontend/public/components/namespace.jsx Outdated Show resolved Hide resolved

sahil143 suggested changes Jun 27, 2019

View reviewed changes

frontend/packages/dev-console/src/components/HintBlock.tsx Outdated Show resolved Hide resolved

frontend/packages/dev-console/src/components/HintBlock.tsx Outdated Show resolved Hide resolved

frontend/packages/dev-console/src/components/HintBlock.tsx Outdated Show resolved Hide resolved

karthikjeeyar force-pushed the landing-page branch from 4f4bb72 to 2436ec2 Compare June 27, 2019 14:15

christianvogt reviewed Jun 28, 2019

View reviewed changes

karthikjeeyar force-pushed the landing-page branch from fb61783 to 10f7375 Compare June 28, 2019 17:54

spadgett reviewed Jun 28, 2019

View reviewed changes

karthikjeeyar force-pushed the landing-page branch 2 times, most recently from 0476f76 to 6104968 Compare June 28, 2019 19:23

karthikjeeyar force-pushed the landing-page branch from 6104968 to 956bc50 Compare June 28, 2019 20:21

christianvogt reviewed Jul 2, 2019

View reviewed changes

openshift-ci-robot assigned rohitkrai03 Jul 3, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2019

christianvogt reviewed Jul 3, 2019

View reviewed changes

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2019

karthikjeeyar commented Jul 3, 2019

View reviewed changes

karthikjeeyar force-pushed the landing-page branch from 4837c0b to 55cc311 Compare July 3, 2019 18:50

openshift-ci-robot assigned christianvogt Jul 3, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2019

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 3, 2019

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 4, 2019

karthikjeeyar force-pushed the landing-page branch from 3d60748 to 28caf19 Compare July 9, 2019 13:44

karthikjeeyar force-pushed the landing-page branch from 28caf19 to 6d24d9a Compare July 9, 2019 14:00

christianvogt reviewed Jul 9, 2019

View reviewed changes

christianvogt reviewed Jul 10, 2019

View reviewed changes

karthikjeeyar force-pushed the landing-page branch from cdc16fa to a73f48f Compare July 10, 2019 15:27

karthik added 2 commits July 10, 2019 23:39

add project list as a default landing page

0af8160

add perspective property to the page route extension

0a967a4

karthikjeeyar force-pushed the landing-page branch from a73f48f to 0a967a4 Compare July 10, 2019 18:09

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 10, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 10, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 10, 2019

openshift-merge-robot merged commit ae1e69a into openshift:master Jul 10, 2019

update empty state secondary masthead flows #1836

update empty state secondary masthead flows #1836

Conversation

karthikjeeyar commented Jun 26, 2019 • edited

sahil143 left a comment

Choose a reason for hiding this comment

sahil143 commented Jun 27, 2019

karthikjeeyar commented Jun 27, 2019

karthikjeeyar commented Jun 27, 2019

karthikjeeyar commented Jun 27, 2019

serenamarie125 commented Jun 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

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

Choose a reason for hiding this comment

karthikjeeyar commented Jun 28, 2019

karthikjeeyar commented Jul 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohitkrai03 commented Jul 3, 2019

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

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

christianvogt commented Jul 3, 2019

christianvogt commented Jul 3, 2019

christianvogt commented Jul 4, 2019

christianvogt commented Jul 4, 2019

karthikjeeyar commented Jul 9, 2019

karthikjeeyar commented Jul 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christianvogt Jul 10, 2019 • edited

Choose a reason for hiding this comment

karthikjeeyar commented Jul 10, 2019

christianvogt commented Jul 10, 2019

christianvogt commented Jul 10, 2019

openshift-ci-robot commented Jul 10, 2019

karthikjeeyar commented Jun 26, 2019 •

edited

christianvogt Jul 10, 2019 •

edited