Dev console #1721

christianvogt · 2019-06-13T20:58:55Z

Contributes the dev console package as a plugin extension.

Note: there are many comments on this PR. Most have been addressed. Then any comment that requires larger changes will be taken up as separate bugs logged against the dev console component in Jira

There are several changes to console that have been broken into small commits. Most of which could be contributed as separate PRs.

Commits

add action item feature to dropdown
add knative icon
- new icon for knative
update eslint rules
- adjusted several rules to avoid conflicts with tsx and existing styles
add perspective header to main nav
- new nav header indicates perspective and cluster hostname
support redirect to perspective landing page
- fix for perspectives to redirect to their default landing page when going to /
update vscode settings for tsdk and debug.node.autoAttach
- save settings in project for those using vscode
update types for page extensions
- fixing typescript errors when adopting the new pages extension
change Administrator perspective icon to PF CogsIcon
- align icon with UX
support children in namespace bar
- dev console needs to add an application dropdown to the namespace bar. This change lets dev console render the namespace bar manually with new content.
support namespace redirect for any URL that supports namespaces
- Loosen the requirement for URLs to be contributed to a static list of namespaced pages. If the URL parses like it supports namespaces, then allow the namespace bar to update the namespace.
add active application redux state, action, reducer
- adding support for active application context in a similar manner that has been done with the active namespace
- this is in console because there are shared screens in admin and dev perspectives that need this
update ownerReference types
- updating types to avoid errors
export routes#getRouteWebURL
- exporting functionality that's used in dev console
export create-secret#SSHAuthSubform
- exporting functionality that's used in dev console
add dev-console dependencies
- adding new dependencies to root package.json
support highlighting active root nav items outside of a section
contribute dev-console plugin
- this commit contains all of the dev console package
enable dev-console plugin in console-app
- adds dev-console package dependency to console-app

fyi @rohitkrai03 @vojtechszocs @spadgett

spadgett · 2019-06-14T13:45:29Z

Thanks @christianvogt. Exciting to see this!

Some things I noticed clicking around:

The experience seems weird when there are no applications:

Somehow I got the wrong content when clicking the topology nav item. (Not sure how I got in this state.)

spadgett

Thanks @christianvogt! A few initial comments. I haven't dug into the dev console itself yet.

frontend/public/components/utils/dropdown.jsx

frontend/public/components/utils/cluster-name.ts

frontend/public/components/nav/nav-header.tsx

frontend/public/components/_dropdown.scss

spadgett · 2019-06-14T12:59:09Z

frontend/public/components/_dropdown.scss

@@ -180,7 +181,7 @@ $dropdown-link-hover-border: 1px solid #bee1f4;
 }

 .co-namespace-selector {
-  max-width: 60%;
+  max-width: 100%;


@rhamilto Do you know why this was 60% before?

This was 60% before because when on mobile it would shrink the project dropdown and use ellipsis to keep it and the import yaml button on a single line.
With the addition of the app selector trying to fit all on one line is not feasible.
What I could do actually is add a class to change the behavior only for when the app selector is included in this bar. I would pass in an additional className to apply for our use case.

That way we maintain the original behavior:

While with the app selector we get this:

spadgett · 2019-06-14T13:14:57Z

frontend/package.json

    "file-saver": "1.3.x",
    "font-awesome": "4.7.x",
+    "formik": "2.0.1-rc.1",


We're locking to a prerelease version?

@spadgett We are using V2 because it comes with support for custom formik hooks and it makes creating custom components wrapped around formik context much easier. They're actually at 2.0.1-rc..5 but because of a bug related to validations i had to downgrade to rc-1. I'll keep track of it and update the version to latest as soon as there's fix.

spadgett · 2019-06-14T13:56:59Z

I saw this error trying to open the "Application" dropdown in the Git form.

Uncaught Invariant Violation: Objects are not valid as a React child (found: object with keys {name}). If you meant to render a collection of children, use an array instead.
    in a (created by DropDownRow)
    in li (created by DropDownRow)
    in DropDownRow (created by Dropdown)
    in ul (created by Dropdown)
    in div (created by Dropdown)
    in div (created by Dropdown)
    in Dropdown (created by ResourceDropdown)
    in ResourceDropdown (created by ApplicationDropdown)
    in Unknown (created by Connect(Component))
    in Connect(Component) (created by Firehose)
    in Firehose (created by Connect(Firehose))
    in Connect(Firehose) (created by ApplicationDropdown)
    in ApplicationDropdown (created by ApplicationSelector)
    in div (created by FormGroup)
    in FormGroup (created by ApplicationSelector)
    in ApplicationSelector (created by AppSection)
    in FormSection (created by AppSection)
    in AppSection (created by GitImportForm)
    in div (created by GitImportForm)
    in form (created by Form)
    in Form (created by GitImportForm)
    in GitImportForm (created by B)
    in B (created by GitImport)
    in GitImport (created by ImportPage)
    in Unknown (created by Connect(Component))
    in Connect(Component) (created by Firehose)
    in Firehose (created by Connect(Firehose))
    in Connect(Firehose) (created by ImportPage)
    in div (created by ImportPage)
    in ImportPage (created by AsyncComponent)
    in AsyncComponent (created by Route)
    in Route (created by LazyRoute)
    in LazyRoute
    in Switch
    in div
    in div
    in section (created by PageSection)
    in PageSection
    in Unknown (created by Route)
    in Route (created by withRouter(undefined))
    in withRouter(undefined) (created by App)
    in main (created by Page)
    in div (created by Page)
    in Page (created by App)
    in App (created by Route)
    in Route
    in Router
    in Provider
    at invariant (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:261812:15)
    at throwOnInvalidObjectType (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:273590:5)
    at reconcileChildFibers (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:274364:7)
    at reconcileChildren (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:276159:28)
    at updateHostComponent (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:276620:3)
    at beginWork (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:277406:14)
    at performUnitOfWork (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:281069:12)
    at workLoop (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:281109:24)
    at HTMLUnknownElement.callCallback (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:261906:14)
    at Object.invokeGuardedCallbackDev (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:261956:16)
ws-factory.js:250 destroying websocket: /api/kubernetes/apis/config.openshift.io/v1/clusterversions?watch=true&fieldSelector=metadata.name%3Dversion
ws-factory.js:250 destroying websocket: /api/kubernetes/apis/image.openshift.io/v1/imagestreams?watch=true&resourceVersion=413787
ws-factory.js:250 destroying websocket: /api/kubernetes/apis/project.openshift.io/v1/projects?watch=true
ws-factory.js:250 destroying websocket: /api/kubernetes/apis/apps.openshift.io/v1/namespaces/viraj/deploymentconfigs?watch=true&resourceVersion=413790
ws-factory.js:250 destroying websocket: /api/kubernetes/apis/apps/v1/namespaces/viraj/deployments?watch=true&resourceVersion=413790
react-dom.development.js:17117 The above error occurred in the <a> component:
    in a (created by DropDownRow)
    in li (created by DropDownRow)
    in DropDownRow (created by Dropdown)
    in ul (created by Dropdown)
    in div (created by Dropdown)
    in div (created by Dropdown)
    in Dropdown (created by ResourceDropdown)
    in ResourceDropdown (created by ApplicationDropdown)
    in Unknown (created by Connect(Component))
    in Connect(Component) (created by Firehose)
    in Firehose (created by Connect(Firehose))
    in Connect(Firehose) (created by ApplicationDropdown)
    in ApplicationDropdown (created by ApplicationSelector)
    in div (created by FormGroup)
    in FormGroup (created by ApplicationSelector)
    in ApplicationSelector (created by AppSection)
    in FormSection (created by AppSection)
    in AppSection (created by GitImportForm)
    in div (created by GitImportForm)
    in form (created by Form)
    in Form (created by GitImportForm)
    in GitImportForm (created by B)
    in B (created by GitImport)
    in GitImport (created by ImportPage)
    in Unknown (created by Connect(Component))
    in Connect(Component) (created by Firehose)
    in Firehose (created by Connect(Firehose))
    in Connect(Firehose) (created by ImportPage)
    in div (created by ImportPage)
    in ImportPage (created by AsyncComponent)
    in AsyncComponent (created by Route)
    in Route (created by LazyRoute)
    in LazyRoute
    in Switch
    in div
    in div
    in section (created by PageSection)
    in PageSection
    in Unknown (created by Route)
    in Route (created by withRouter(undefined))
    in withRouter(undefined) (created by App)
    in main (created by Page)
    in div (created by Page)
    in Page (created by App)
    in App (created by Route)
    in Route
    in Router
    in Provider

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://fb.me/react-error-boundaries to learn more about error boundaries.
logCapturedError @ react-dom.development.js:17117
react-dom.development.js:20312 Uncaught Invariant Violation: Objects are not valid as a React child (found: object with keys {name}). If you meant to render a collection of children, use an array instead.
    in a (created by DropDownRow)
    in li (created by DropDownRow)
    in DropDownRow (created by Dropdown)
    in ul (created by Dropdown)
    in div (created by Dropdown)
    in div (created by Dropdown)
    in Dropdown (created by ResourceDropdown)
    in ResourceDropdown (created by ApplicationDropdown)
    in Unknown (created by Connect(Component))
    in Connect(Component) (created by Firehose)
    in Firehose (created by Connect(Firehose))
    in Connect(Firehose) (created by ApplicationDropdown)
    in ApplicationDropdown (created by ApplicationSelector)
    in div (created by FormGroup)
    in FormGroup (created by ApplicationSelector)
    in ApplicationSelector (created by AppSection)
    in FormSection (created by AppSection)
    in AppSection (created by GitImportForm)
    in div (created by GitImportForm)
    in form (created by Form)
    in Form (created by GitImportForm)
    in GitImportForm (created by B)
    in B (created by GitImport)
    in GitImport (created by ImportPage)
    in Unknown (created by Connect(Component))
    in Connect(Component) (created by Firehose)
    in Firehose (created by Connect(Firehose))
    in Connect(Firehose) (created by ImportPage)
    in div (created by ImportPage)
    in ImportPage (created by AsyncComponent)
    in AsyncComponent (created by Route)
    in Route (created by LazyRoute)
    in LazyRoute
    in Switch
    in div
    in div
    in section (created by PageSection)
    in PageSection
    in Unknown (created by Route)
    in Route (created by withRouter(undefined))
    in withRouter(undefined) (created by App)
    in main (created by Page)
    in div (created by Page)
    in Page (created by App)
    in App (created by Route)
    in Route
    in Router
    in Provider
    at invariant (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:261812:15)
    at throwOnInvalidObjectType (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:273590:5)
    at reconcileChildFibers (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:274364:7)
    at reconcileChildren (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:276159:28)
    at updateHostComponent (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:276620:3)
    at beginWork (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:277406:14)
    at performUnitOfWork (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:281069:12)
    at workLoop (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:281109:24)
    at renderRoot (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:281192:7)
    at performWorkOnRoot (http://localhost:9000/static/vendors~main-6b817cfa623d852528dd.js:282099:7)

spadgett · 2019-06-14T14:18:35Z

Expand/collapse sections in forms can get confusing. We had some usability bugs on that in the past.

It's not clear if you edit something and collapse the section whether it still takes effect
How do you handle invalid fields in collapsed sections?

(Not saying we need to change it in this PR, just giving feedback from past experience.)

spadgett · 2019-06-14T14:21:08Z

I see now that the topology view is meant to show the "add" options as an empty state. That's super confusing imo. I thought it was a bug! We should at least add a message saying there are no workloads in your project.

christianvogt · 2019-06-14T14:59:11Z

@spadgett the expand collapse of sections in the form is temporary while we work on a new design from UX this sprint.

christianvogt · 2019-06-14T15:00:46Z

@spadgett with regards to the topology view showing the empty state. That is currently changing. I'll share the design doc with you.
There is a nav bug i see where the current item isn't being highlighted because we use top level nav items and the logic is only available in sections today. I'll have to address that.

spadgett · 2019-06-14T14:27:29Z

frontend/packages/dev-console/src/components/dropdown/ResourceDropdown.tsx

+  onChange?: (name: string, key: string) => void;
+}
+
+class ResourceDropdown extends React.Component<ResourceDropdownProps, State> {


Note that we have an existing ResourceListDropdown. We should look at the differences and try to avoid having multiple components that do the same thing.

spadgett · 2019-06-14T14:28:22Z

frontend/packages/dev-console/src/components/dropdown/ResourceDropdown.tsx

+  buttonClassName?: string;
+  title?: React.ReactNode;
+  titlePrefix?: string;
+  allApplicationsKey?: string;


Looks like there are props here that aren't used?

spadgett · 2019-06-14T14:33:07Z

frontend/packages/dev-console/src/components/formik-fields/CheckboxField.tsx

+      controlId={`${props.name}-field`}
+      validationState={getValidationState(error, touched)}
+    >
+      <Checkbox {...field} {...props} checked={field.value} aria-describedby={`${props.name}-help`}>


aria-describedby will be invalid if there is no helpText

spadgett · 2019-06-14T14:34:01Z

frontend/packages/dev-console/src/components/formik-fields/CheckboxField.tsx

+      <Checkbox {...field} {...props} checked={field.value} aria-describedby={`${props.name}-help`}>
+        {label}
+      </Checkbox>
+      {helpText && <HelpBlock id={`${props.name}-help`}>{helpText}</HelpBlock>}


Consider _.uniqueId to make sure the id is unique on the page.

Unique IDs is something we need to tackle across the app and we should be generating IDs in the underlying form components and at least having a component that helps manage the ID generation such as the one used in PF4. I'd like to tackle this post merge.

spadgett · 2019-06-14T14:36:48Z

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

+            <CardHeader>Import from Git</CardHeader>
+            <CardBody>Import code from your git repository, to be built and deployed </CardBody>
+            <CardFooter>
+              <Link className="pf-c-button pf-m-secondary" to="/import">


We should consider adding RBAC checks. What should the user experience be if I don't have authority to add to the project?

Good point. I've raised your concern with UX. This is something we can add to our backlog for 4.2. We have a story to rework the empty state patterns with new UX. I've added your concerns there.

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

spadgett · 2019-06-14T14:59:20Z

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

+import { ColHead, ListHeader } from '@console/internal/components/factory';
+
+const PipelineHeader = (props) => (
+  <ListHeader>


This should switch to PF4 tables, which recently landed in master.

Damn. Missed the boat lol
I can look at updating our lists....

I wasn't aware that had landed yet - sorry, thought Patrick was still working on those :]

spadgett · 2019-06-14T14:59:40Z

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

+      <div className="col-lg-2 col-md-2 hidden-sm hidden-xs">
+        <Timestamp
+          timestamp={
+            pipeline.latestRun &&


spadgett · 2019-06-14T15:00:04Z

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

+    selected: ['Succeeded'],
+    reducer: pipelineRunFilterReducer,
+    items: [
+      { id: 'Succeeded', title: 'Complete' },


Any reason not to match the title to id?

AFAIK this was done according to UX. I can followup with the dev who works on pipelines.
@serenamarie125

Abhi confirmed this is intentional after discussing the values with UX and the tekton team

yes @spadgett @christianvogt the decision was to match what's in the upstream

frontend/packages/dev-console/src/components/pipelines/PipelineVisualizationStepList.scss

spadgett · 2019-06-14T17:28:29Z

Project bar wrapping is a little awkward at some widths. If nothing else, the bottom margin looks off.

spadgett · 2019-06-14T17:29:18Z

Details sidebar uses different font sizes in the different contexts:

spadgett · 2019-06-14T17:32:05Z

"Create New Application" option has a placeholder style when it shouldn't. (Our Dropdown component has an extremely confusing API. We've talked about transitioning away from it.)

spadgett · 2019-06-14T17:37:24Z

The dev console label selector is inconsistent with other label selectors in console:

vs

and

spadgett · 2019-06-14T18:08:29Z

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

+
+  componentDidMount() {
+    // eslint-disable-next-line promise/catch-or-return
+    k8sGet(PipelineModel, this.props.name, this.props.namespace).then((res) => {


Error handling?

I will create separate bugs for the team to address these.

spadgett · 2019-06-14T18:09:02Z

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

+      }).then((listres) => {
+        this.setState({
+          menuActions: [
+            triggerPipeline(res, getLatestRun({ data: listres }, 'creationTimestamp'), 'pipelines'),


We should add RBAC checks to the menu items. For example,

https://github.com/openshift/console/blob/master/frontend/public/components/utils/kebab.tsx#L65-L78

spadgett · 2019-06-14T18:22:05Z

For new code, we're moving to functional components and hooks instead of class-based components. Pretty sure you're aware and most of this was written before we bumped React versions, but I wanted to mention it.

spadgett · 2019-06-14T18:23:56Z

White tooltip text on a gray background isn't legible:

spadgett · 2019-06-14T18:44:18Z

spadgett · 2019-06-14T18:48:26Z

Import YAML button has different styles in developer console vs admin console:

christianvogt · 2019-06-19T05:46:47Z

rebased

ppitonak · 2019-06-19T12:54:50Z

@christianvogt ./build.sh fails when I rebase

spadgett · 2019-06-19T15:20:03Z

@christianvogt fyi, needs rebase again :/

christianvogt · 2019-06-19T15:30:40Z

I just rebased everything again :)

christianvogt · 2019-06-19T15:44:05Z

@christianvogt ./build.sh fails when I rebase

@ppitonak I just and it works for me

spadgett · 2019-06-19T17:03:40Z

CI will fail if build.sh fails, so we should be good

spadgett

Let's get this integrated and address any additional issues in follow-up PRs. Thanks @christianvogt 👍

/lgtm

openshift-ci-robot · 2019-06-19T17:05:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, divyanshiGupta, karthikjeeyar, rohitkrai03, spadgett, vikram-raj

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~~ [spadgett]

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 do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 13, 2019

openshift-ci-robot requested review from jcaianirh and spadgett June 13, 2019 20:59

christianvogt force-pushed the dev-console branch 2 times, most recently from a6e9b58 to e3e5564 Compare June 14, 2019 02:45

spadgett reviewed Jun 14, 2019

View reviewed changes

christianvogt force-pushed the dev-console branch from e3e5564 to d05419b Compare June 14, 2019 14:47

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

spadgett reviewed Jun 14, 2019

View reviewed changes

christianvogt force-pushed the dev-console branch from d05419b to e7df530 Compare June 14, 2019 15:44

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

christianvogt force-pushed the dev-console branch from e7df530 to 8c71f1a Compare June 14, 2019 17:35

spadgett mentioned this pull request Jun 14, 2019

Add details card #1696

Merged

christianvogt force-pushed the dev-console branch from 8c71f1a to 8a58604 Compare June 14, 2019 18:12

spadgett reviewed Jun 14, 2019

View reviewed changes

christianvogt added 14 commits June 19, 2019 10:44

add action item feature to dropdown

f0ac272

add knative icon

79b4c1a

update eslint rules

83c4d9f

add perspective header to main nav

51ec09a

support redirect to perspective landing page

d0521ac

update vscode settings for tsdk and debug.node.autoAttach

c07522c

update types for page extensions

ed91470

change Administrator perspective icon to PF CogsIcon

1738dcd

support children in namespace bar

ad6606d

support namespace redirect for any URL that supports namespaces

c5f64a1

add active application redux state, action, reducer

f2eca9d

update ownerReference types

2701ab3

export routes#getRouteWebURL

b48c7b0

export create-secret#SSHAuthSubform

6dcf8a1

christianvogt added 4 commits June 19, 2019 11:28

support highlighting active root nav items outside of a section

0998cb7

add dev-console dependencies

a1fef27

contribute dev-console plugin

4a74c4b

enable dev-console plugin in console-app

382e7fb

christianvogt force-pushed the dev-console branch from 76e0d39 to 382e7fb Compare June 19, 2019 15:28

spadgett approved these changes Jun 19, 2019

View reviewed changes

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

openshift-merge-robot merged commit c2260b6 into openshift:master Jun 19, 2019

christianvogt deleted the dev-console branch August 20, 2020 16:31

Dev console #1721

Dev console #1721

Conversation

christianvogt commented Jun 13, 2019 • edited Loading

Commits

spadgett commented Jun 14, 2019

spadgett left a 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

spadgett commented Jun 14, 2019

spadgett commented Jun 14, 2019

spadgett commented Jun 14, 2019

christianvogt commented Jun 14, 2019

christianvogt commented Jun 14, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett commented Jun 14, 2019

spadgett commented Jun 14, 2019

spadgett commented Jun 14, 2019

spadgett commented Jun 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett commented Jun 14, 2019

spadgett commented Jun 14, 2019

spadgett commented Jun 14, 2019

spadgett commented Jun 14, 2019

christianvogt commented Jun 19, 2019

ppitonak commented Jun 19, 2019

spadgett commented Jun 19, 2019

christianvogt commented Jun 19, 2019

christianvogt commented Jun 19, 2019

spadgett commented Jun 19, 2019

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jun 19, 2019

christianvogt commented Jun 13, 2019 •

edited

Loading

spadgett commented Jun 14, 2019 •

edited

Loading