fix(PipelineRunList) - Make Pipeline Run List to be consistent and add dev preview badge. by abhinandan13jan · Pull Request #2819 · openshift/console

abhinandan13jan · 2019-09-24T12:43:03Z

Addresses
https://jira.coreos.com/browse/ODC-1925
https://jira.coreos.com/browse/ODC-1924

rohitkrai03 · 2019-10-01T08:47:22Z

/approve

andrewballantyne

There are some duplication issues at play here.

andrewballantyne · 2019-10-01T17:09:17Z

I am not sure about just selecting Succeeded... I've reached out to UX. Personally I don't like it, so I want to make sure it's intentional and desired we mimic the logic from the pipeline -> pipeline-run page.

cc @serenamarie125

@abhinandan13jan I've got confirmation that we want all of them selected. Could you make this change to this page as well as the Pipeline's PipelineRun Page? Ideally, making use of the same data-object between the two.

cc @serenamarie125

Updated this

andrewballantyne · 2019-10-01T17:10:06Z

We are duplicating filters. frontend/packages/dev-console/src/components/pipelines/PipelineRuns.tsx has the same filters.

Making sure, the same filter is used in both places

andrewballantyne · 2019-10-01T17:14:25Z

I think we need to start making concerted efforts to avoid any id or reference value being a magic string. It's not great for maintenance moving forward. I'd like to see an enum be used for the filter ids and in a common file that can be used for other filters that match this design.

For instance - there is similar logic found here: frontend/packages/dev-console/src/components/pipelines/PipelineAugmentRuns.tsx

Using enums now

andrewballantyne · 2019-10-01T17:16:06Z

/assign @andrewballantyne

andrewballantyne · 2019-10-09T13:23:22Z

~~Can we actually get rid of this page? #2917 will cover adding this badge generically without the need for an additional file.~~

Doh - nvm, we need this page for the filters. Whomever gets in last (this PR or #2917) will need to merge in the getBadgeFromType logic.

andrewballantyne · 2019-10-09T13:34:01Z

@abhinandan13jan you changed the label. Could you undo that?

andrewballantyne · 2019-10-09T13:37:45Z

Perhaps what we can do here is make a map instead of an enum. We could us the ListFilterIds as keys in the map so show they are related. Also, ListFilterStatuses is probably better named listFilterLabels as they are labels.

Something like:

export const listFilterLabels = { [ListFilterIds.Running]: 'Running', [ListFilterIds.Failed]: 'Failed', [ListFilterIds.Succeeded]: 'Complete', // this was the old label noted in a previous comment };

andrewballantyne · 2019-10-09T13:38:31Z

RunFilters could probably be named runFilters as it's not a class/component.

Suggested change

export const RunFilters = [

export const runFilters = [

andrewballantyne · 2019-10-09T13:39:53Z

I believe enums are supposed to be singular.

Suggested change

export enum ListFilterIds {

export enum ListFilterId {

andrewballantyne · 2019-10-10T12:56:07Z

The order is not consistent. Perhaps we make items a variable somewhere else? We'd want the filter items to be in the same order throughout the pipelines otherwise the user instinctively won't be able to click on filters without stopping to read what order they happen to be in on a given page.

Sorry I missed this on my previous review passes.

Updated to have a consistent order throughout

andrewballantyne · 2019-10-10T13:49:52Z

/lgtm

openshift-ci-robot · 2019-10-10T13:50:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, andrewballantyne, 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:

~~frontend/packages/dev-console/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-robot · 2019-10-10T13:50:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, andrewballantyne, 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:

~~frontend/packages/dev-console/OWNERS~~ [rohitkrai03]

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

andrewballantyne · 2019-10-10T15:07:29Z

/test e2e-aws

andrewballantyne · 2019-10-10T15:31:47Z

/test e2e-aws

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/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 24, 2019

openshift-ci-robot requested review from nemesis09 and sahil143 September 24, 2019 12:43

openshift-ci-robot added the component/dev-console Related to dev-console label Sep 24, 2019

abhinandan13jan changed the title ~~[WIP]fix(PipelineRunList) - Make Pipeline Run List to be consistent and add dev preview badge.~~ fix(PipelineRunList) - Make Pipeline Run List to be consistent and add dev preview badge. Oct 1, 2019

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 Oct 1, 2019

rohitkrai03 approved these changes Oct 1, 2019

View reviewed changes

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

andrewballantyne suggested changes Oct 1, 2019

View reviewed changes

openshift-ci-robot assigned andrewballantyne Oct 1, 2019

andrewballantyne mentioned this pull request Oct 4, 2019

Adds Resource Model Badge information #2917

Merged

abhinandan13jan force-pushed the prun-devbadge branch from 4061560 to 5e6fcf6 Compare October 9, 2019 10:28

andrewballantyne reviewed Oct 9, 2019

View reviewed changes

abhinandan13jan force-pushed the prun-devbadge branch from 5e6fcf6 to a2cb6a8 Compare October 10, 2019 12:33

andrewballantyne reviewed Oct 10, 2019

View reviewed changes

fix(PipelineRunList) Updating resource page and adding Dev preview badge

78c7dea

abhinandan13jan force-pushed the prun-devbadge branch from a2cb6a8 to 78c7dea Compare October 10, 2019 13:22

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

andrewballantyne approved these changes Oct 10, 2019

View reviewed changes

openshift-merge-robot merged commit f5b20d1 into openshift:master Oct 10, 2019

spadgett added this to the v4.3 milestone Oct 10, 2019

Uh oh!

Conversation

abhinandan13jan commented Sep 24, 2019

Addresses https://jira.coreos.com/browse/ODC-1925 https://jira.coreos.com/browse/ODC-1924

Uh oh!

rohitkrai03 commented Oct 1, 2019

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!

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 Oct 1, 2019

Uh oh!

andrewballantyne Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewballantyne Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewballantyne Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

andrewballantyne commented Oct 10, 2019

Uh oh!

openshift-ci-robot commented Oct 10, 2019

Uh oh!

openshift-ci-robot commented Oct 10, 2019

Uh oh!

andrewballantyne commented Oct 10, 2019

Uh oh!

andrewballantyne commented Oct 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Addresses
https://jira.coreos.com/browse/ODC-1925
https://jira.coreos.com/browse/ODC-1924

andrewballantyne Oct 9, 2019 •

edited

Loading

andrewballantyne Oct 9, 2019 •

edited

Loading

andrewballantyne Oct 9, 2019 •

edited

Loading