Skip to content

Conversation

@andrewballantyne
Copy link
Contributor

Fixes:
Dependency on #4004 (https://issues.redhat.com/browse/ODC-2605) -- 1st commit
Dependency on #4031 (https://issues.redhat.com/browse/ODC-2448) -- 2nd commit
https://issues.redhat.com/browse/ODC-2447 -- 3rd commit

Do review of the third commit while we wait for #4004 & #4031 to get in.

Analysis / Root cause:
With the work done in #4004 and #4031 we needed a real user page to make use of the Pipeline Builder.

Solution Description:
Mount a formik page and form; stitching up the Pipeline Builder visualization from #4031.

Screen shots / Gifs for design review:
PipelineBuilderForm

@openshift/team-devconsole-ux

Unit test coverage report:

  • Unit Tests

Test setup:

  • Install the Pipeline Operator (this comes with cluster tasks from 0.8.x onwards)
  • Go to the Pipelines page and "Create Pipeline"

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

/kind feature
/assign @christianvogt

@openshift-ci-robot openshift-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. component/dev-console Related to dev-console labels Jan 21, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: andrewballantyne
To complete the pull request process, please assign christianvogt
You can assign the PR to them by writing /assign @christianvogt in a comment when ready.

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

Details Needs approval from an approver in each of these files:

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

Copy link
Contributor

@serenamarie125 serenamarie125 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far!
Selecting that task is rough ;) but not your implementation issue

@andrewballantyne
Copy link
Contributor Author

Looking good so far!
Selecting that task is rough ;) but not your implementation issue

I can probably cap the height of the popout - might make it a little nicer to work with.

@abhinandan13jan
Copy link
Contributor

there are lint errors

@andrewballantyne
Copy link
Contributor Author

there are lint errors

@abhinandan13jan There sure are! I'm having issues with my linter and I posted that in the #forum-ui room. Please review around the lint errors :)

edges={edges}
layout={PipelineLayout.DAGRE_VIEWER}
/>
<div style={{ marginBottom: 'var(--pf-global--spacer--md)' }}>
Copy link
Contributor

@abhinandan13jan abhinandan13jan Jan 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not marginBottom: global_spacer_md.value from patternfly/react-tokens? Both would do the same thing... I am unsure which is more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was likely a hack to test something and stayed because it worked... probably should go into a scss file.

@abhinandan13jan
Copy link
Contributor

apart from one small comment, the third commit looks good.

@andrewballantyne andrewballantyne mentioned this pull request Jan 23, 2020
5 tasks
@andrewballantyne
Copy link
Contributor Author

Folded into #4055 for CI sanity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/dev-console Related to dev-console kind/feature Categorizes issue or PR as related to a new feature. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants