Bug 1798417: MultiColumnField should hide headers if there are no rows #4180

abhinandan13jan · 2020-02-04T12:52:00Z

Addresses: https://issues.redhat.com/browse/ODC-2828

Updated Screenshot (Using & adding a margin of spacer-sm|8px)

andrewballantyne · 2020-02-04T14:22:27Z

/kind bug

abhinandan13jan · 2020-02-05T10:08:43Z

linked to bug https://bugzilla.redhat.com/show_bug.cgi?id=1798417

abhinandan13jan · 2020-02-05T10:10:08Z

/retitle Bug 1798417: MultiColumnField should hide headers if there are no rows

openshift-ci-robot · 2020-02-05T10:10:21Z

@abhinandan13jan: This pull request references Bugzilla bug 1798417, which is valid. The bug has been moved to the POST state.

Details

In response to this:

Bug 1798417: MultiColumnField should hide headers if there are no rows

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.

abhinandan13jan · 2020-02-05T10:10:45Z

/bugzilla refresh

openshift-ci-robot · 2020-02-05T10:10:59Z

@abhinandan13jan: This pull request references Bugzilla bug 1798417, which is valid.

Details

In response to this:

/bugzilla refresh

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.

abhinandan13jan · 2020-02-05T10:11:18Z

/test e2e-gcp-console

rohitkrai03

/lgtm

christianvogt · 2020-02-05T12:59:43Z

...packages/console-shared/src/components/formik-fields/multi-column-field/MultiColumnField.tsx

Do not render this is if there is no message.
emptyMessage && <...>

Also is there a pf4 style or component that provides this secondary text styles?

Try using the pf4's Text component here

Not sure Text will work because there's only one variant that has the same secondary color which is small. I don't think we want smaller text here though.

Now, using the SecondaryStatus component

andrewballantyne · 2020-02-05T13:43:13Z

cc @openshift/team-devconsole-ux

andrewballantyne · 2020-02-06T11:45:42Z

The text appears quite close to the link button... make sure you add that spacing @abhinandan13jan - see the screenshots in the Design ticket if you need more information. Feel free to reach out to UX directly as well.

serenamarie125

@abhinandan13jan can you add 8px padding between the empty state message and the + Add link?

serenamarie125

Also checking - this fix is being made for both the Parameters & Resources tab, right?

openshift-ci-robot · 2020-02-07T08:32:32Z

@abhinandan13jan: This pull request references Bugzilla bug 1798417, which is valid.

Details

In response to this:

Bug 1798417: MultiColumnField should hide headers if there are no rows

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.

abhinandan13jan · 2020-02-07T08:34:00Z

@andrewballantyne @serenamarie125 I've added a spacer-sm(8px) between empty message. And Yes @serenamarie125 This would apply to both param & resources. :)

serenamarie125 · 2020-02-07T13:10:57Z

This spacing matches the design, thanks for making that update @abhinandan13jan

bgliwa01 · 2020-02-07T13:13:06Z

@abhinandan13jan @andrewballantyne if this is a utility make sure this text is used- for Pipelines it should be "Add Pipeline Parameter" and for Resources it should be "Add Pipeline Resource"

andrewballantyne · 2020-02-07T16:13:31Z

@abhinandan13jan @andrewballantyne if this is a utility make sure this text is used- for Pipelines it should be "Add Pipeline Parameter" and for Resources it should be "Add Pipeline Resource"

Good feedback @bgliwa01 thanks! @abhinandan13jan make sure you update the button text at the same time.

andrewballantyne

Found the spots, please update @abhinandan13jan

andrewballantyne · 2020-02-07T16:15:12Z

frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/PipelineParameters.tsx

Adjust to fit with UX text.

andrewballantyne · 2020-02-07T16:15:34Z

frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/PipelineResources.tsx

Adjust to fit with UX text. Should be singular.

abhinandan13jan · 2020-02-10T12:01:42Z

@bgliwa01 @andrewballantyne Updated the text

debsmita1 · 2020-02-10T14:28:12Z

frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/PipelineParameters.tsx

Suggested change

const emptyMessage = 'No params are associated with this pipeline';

const emptyMessage = 'No parameters are associated with this pipeline.';

debsmita1 · 2020-02-10T14:28:39Z

frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/PipelineResources.tsx

Suggested change

const emptyMessage = 'No resources are associated with this pipeline';

const emptyMessage = 'No resources are associated with this pipeline.';

debsmita1 · 2020-02-10T14:31:04Z

...ackages/console-shared/src/components/formik-fields/multi-column-field/MultiColumnField.scss

Suggested change

&__emptymsg{

&__empty-message {

debsmita1 · 2020-02-11T11:44:55Z

/lgtm

andrewballantyne · 2020-02-11T13:14:19Z

...packages/console-shared/src/components/formik-fields/multi-column-field/MultiColumnField.tsx

+          {fieldValue.length < 1 ? (
+            emptyMessage && (
+              <div className="odc-multi-column-field__empty-message">
+                <SecondaryStatus status={emptyMessage} />
+              </div>
+            )
+          ) : (
+            <MultiColumnFieldHeader headers={headers} />
+          )}


@openshift/team-devconsole-ux

As it stands, without an "empty message" will remove the headers. I assume this desirable?

With content:

Empty (although this is not an ideal state for this page to be in):

We use this component in 4 spots

Pipeline Parameters

Pipeline Resources

Project Access

Knative Traffic Split

We'll need to come up with how we want to handle those. I logged https://issues.redhat.com/browse/ODC-2976 a while back to address chasing them down and asking this question.

christianvogt · 2020-02-11T19:53:37Z

/approve

openshift-ci-robot · 2020-02-11T19:53:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, christianvogt, debsmita1, rohitkrai03, serenamarie125

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:

~~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 · 2020-02-11T23:09:32Z

@abhinandan13jan: All pull requests linked via external trackers have merged. Bugzilla bug 1798417 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1798417: MultiColumnField should hide headers if there are no rows

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.

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/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 4, 2020

openshift-ci-robot requested review from spadgett and vikram-raj February 4, 2020 12:52

openshift-ci-robot added component/dev-console Related to dev-console component/shared Related to console-shared labels Feb 4, 2020

openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Feb 4, 2020

abhinandan13jan changed the title ~~[WIP]fix(multicolumn-field): Remove header for empty case~~ fix(multicolumn-field): Remove header for empty case Feb 5, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 5, 2020

openshift-ci-robot changed the title ~~fix(multicolumn-field): Remove header for empty case~~ Bug 1798417: MultiColumnField should hide headers if there are no rows Feb 5, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Feb 5, 2020

rohitkrai03 approved these changes Feb 5, 2020

View reviewed changes

openshift-ci-robot assigned rohitkrai03 Feb 5, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 5, 2020

christianvogt reviewed Feb 5, 2020

View reviewed changes

andrewballantyne mentioned this pull request Feb 7, 2020

Bug 1801857: Cleanup Pipeline Builder #4240

Merged

4 tasks

serenamarie125 suggested changes Feb 7, 2020

View reviewed changes

serenamarie125 reviewed Feb 7, 2020

View reviewed changes

abhinandan13jan force-pushed the empty-multi-column branch from 5db1690 to 9feb0b5 Compare February 7, 2020 08:11

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 7, 2020

abhinandan13jan force-pushed the empty-multi-column branch from 9feb0b5 to 5c0604b Compare February 7, 2020 08:29

andrewballantyne suggested changes Feb 7, 2020

View reviewed changes

abhinandan13jan force-pushed the empty-multi-column branch from 5c0604b to 7763efe Compare February 10, 2020 08:23

debsmita1 reviewed Feb 10, 2020

View reviewed changes

serenamarie125 approved these changes Feb 11, 2020

View reviewed changes

fix(multicolumn-field): Remove header for empty case

ab9573d

abhinandan13jan force-pushed the empty-multi-column branch from 7763efe to ab9573d Compare February 11, 2020 08:06

openshift-ci-robot assigned debsmita1 Feb 11, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 11, 2020

andrewballantyne reviewed Feb 11, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 11, 2020

openshift-merge-robot merged commit 8f149ac into openshift:master Feb 11, 2020

spadgett added this to the v4.4 milestone Feb 17, 2020

	const emptyMessage = 'No params are associated with this pipeline';
	const emptyMessage = 'No parameters are associated with this pipeline.';

	const emptyMessage = 'No resources are associated with this pipeline';
	const emptyMessage = 'No resources are associated with this pipeline.';

Bug 1798417: MultiColumnField should hide headers if there are no rows #4180

Bug 1798417: MultiColumnField should hide headers if there are no rows #4180

Uh oh!

Conversation

abhinandan13jan commented Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewballantyne commented Feb 4, 2020

Uh oh!

abhinandan13jan commented Feb 5, 2020

Uh oh!

abhinandan13jan commented Feb 5, 2020

Uh oh!

openshift-ci-robot commented Feb 5, 2020

Uh oh!

abhinandan13jan commented Feb 5, 2020

Uh oh!

openshift-ci-robot commented Feb 5, 2020

Uh oh!

abhinandan13jan commented Feb 5, 2020

Uh oh!

rohitkrai03 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewballantyne commented Feb 5, 2020

Uh oh!

andrewballantyne commented Feb 6, 2020

Uh oh!

serenamarie125 left a comment

Choose a reason for hiding this comment

Uh oh!

serenamarie125 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Feb 7, 2020

Uh oh!

abhinandan13jan commented Feb 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serenamarie125 commented Feb 7, 2020

Uh oh!

bgliwa01 commented Feb 7, 2020

Uh oh!

andrewballantyne commented Feb 7, 2020

Uh oh!

andrewballantyne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhinandan13jan commented Feb 10, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

debsmita1 commented Feb 11, 2020

Uh oh!

abhinandan13jan commented Feb 4, 2020 •

edited

Loading

abhinandan13jan commented Feb 7, 2020 •

edited

Loading

andrewballantyne Feb 11, 2020 •

edited

Loading