Bug 1875809: Restyle Time to inlineblock for Topology - Pipeline & Builds #6493

abhinandan13jan · 2020-09-02T11:43:09Z

Issue

https://issues.redhat.com/browse/ODC-4455

Solution

Fixed in two different commits

Make PipelineOverview time as inline-block
Make BuildOverview block as inline-block

Screenshot

Tests

No tests altered

Browser conformance

Chrome & Firefox

andrewballantyne · 2020-09-03T15:12:45Z

frontend/packages/dev-console/src/components/pipelines/pipeline-overview/PipelineRunItem.scss

@@ -0,0 +1,5 @@
+.pipeline-run-item {


Suggested change

.pipeline-run-item {

.odc-pipeline-run-item {

andrewballantyne · 2020-09-03T15:13:38Z

frontend/packages/dev-console/src/components/pipelines/pipeline-overview/PipelineRunItem.tsx

@@ -31,7 +32,11 @@ const PipelineRunItem: React.FC<PipelineRunItemProps> = ({ pipelineRun }) => {
      <Grid hasGutter>


The li one line up could use with the odc-pipeline-run-item class too to maintain a good structure.

andrewballantyne · 2020-09-03T15:14:36Z

frontend/public/components/overview/_build-overview.scss

@@ -9,6 +9,10 @@
  align-items: center;
 }

+.build-overview__item-time {


I think we can probably leave the build class alone without the prefix... it's older code and appears to be more consistent with what is there.

I think something got lost in translation here @abhinandan13jan haha... I didn't mean undo this change, I was merely saying don't change .build-overview__item-time to meet my other change request. We definitely still want to apply this to Builds (I'm not contradicting myself from the previous time you asked about addressing the Build Config section -- we still definitely want to keep this functionality consistent).

openshift-ci-robot · 2020-09-04T12:36:41Z

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

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1875809: Restyle Time to inlineblock for Topology - Pipeline & Builds

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-09-04T12:36:51Z

/kind bug

abhinandan13jan · 2020-09-04T12:36:59Z

@andrewballantyne updated

andrewballantyne

The Pipeline section changes look good ... please return the changes for the Build section. Apologizes for the misunderstanding.

abhinandan13jan · 2020-09-08T09:13:12Z

/test frontend

andrewballantyne · 2020-09-09T14:16:52Z

/retest

andrewballantyne · 2020-09-09T14:17:37Z

/lgtm
/assign @rohitkrai03

christianvogt · 2020-09-09T16:21:03Z

frontend/packages/dev-console/src/components/pipelines/pipeline-overview/PipelineRunItem.tsx

-          {lastUpdated && <span className="text-muted">&nbsp;({fromNow(lastUpdated)})</span>}
+          {lastUpdated && (
+            <span className="odc-pipeline-run-item__time text-muted">
+              &nbsp;({fromNow(lastUpdated)})


We shouldn't be using   anymore because when the time wraps, it creates an unwanted space offset at the beginning of the line.

You can see this in the description screenshot.

removed nbsp from both Pipeline and Build

christianvogt · 2020-09-10T18:54:57Z

frontend/packages/dev-console/src/components/pipelines/pipeline-overview/PipelineRunItem.tsx

+          {lastUpdated && (
+            <span className="odc-pipeline-run-item__time text-muted">({fromNow(lastUpdated)})</span>
+          )}


By removing   you've removed the separation between text:

You need to use {' '}

Doesn't work with the space, it gets trimmed off. I tried to achieve the behavior by using a right-padding on previous element.
Let me know if that's fine.

christianvogt · 2020-09-11T16:24:58Z

frontend/public/components/overview/build-overview.tsx

@@ -44,8 +44,10 @@ const BuildOverviewItem: React.SFC<BuildOverviewListItemProps> = ({ build }) =>
      Build &nbsp;
      <BuildNumberLink build={build} />
      &nbsp;
-      {conjugateBuildPhase(phase)}
-      {lastUpdated && <span className="text-muted">&nbsp;({fromNow(lastUpdated)})</span>}
+      <span className="build-overview__item-phase">{conjugateBuildPhase(phase)}</span>


What about doing this:

<div> Build <BuildNumberLink build={build} /> {conjugateBuildPhase(phase)} {lastUpdated && ( <> {' '} <span className="build-overview__item-time text-muted">({fromNow(lastUpdated)})</span> </> )} </div>

Updated for both Builds and Pipelines

christianvogt · 2020-09-11T20:18:18Z

/lgtm
/approve

openshift-ci-robot · 2020-09-11T20:18:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, andrewballantyne, christianvogt

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 · 2020-09-11T22:44:18Z

@abhinandan13jan: All pull requests linked via external trackers have merged:

openshift/console#6493

Bugzilla bug 1875809 has been moved to the MODIFIED state.

In response to this:

Bug 1875809: Restyle Time to inlineblock for Topology - Pipeline & Builds

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 the component/core Related to console core functionality label Sep 2, 2020

openshift-ci-robot requested review from dtaylor113 and vikram-raj September 2, 2020 11:43

openshift-ci-robot added the component/dev-console Related to dev-console label Sep 2, 2020

andrewballantyne suggested changes Sep 3, 2020

View reviewed changes

abhinandan13jan force-pushed the time-inline branch from e0afd1c to 7b048cf Compare September 4, 2020 12:19

abhinandan13jan changed the title ~~Restyle Time to inlineblock for Topology - Pipeline & Builds~~ Bug 1875809: Restyle Time to inlineblock for Topology - Pipeline & Builds Sep 4, 2020

openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Sep 4, 2020

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

andrewballantyne suggested changes Sep 4, 2020

View reviewed changes

abhinandan13jan force-pushed the time-inline branch from 7b048cf to de4be08 Compare September 8, 2020 08:47

andrewballantyne approved these changes Sep 9, 2020

View reviewed changes

openshift-ci-robot assigned rohitkrai03 and andrewballantyne Sep 9, 2020

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

christianvogt reviewed Sep 9, 2020

View reviewed changes

abhinandan13jan force-pushed the time-inline branch from de4be08 to 1c17312 Compare September 10, 2020 11:33

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

christianvogt reviewed Sep 10, 2020

View reviewed changes

christianvogt reviewed Sep 11, 2020

View reviewed changes

add inline block style for pipeline overview

3e0fa8b

abhinandan13jan force-pushed the time-inline branch from 8ba8b88 to 3e0fa8b Compare September 11, 2020 17:12

openshift-ci-robot assigned christianvogt Sep 11, 2020

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 11, 2020

openshift-merge-robot merged commit f988ecb into openshift:master Sep 11, 2020

spadgett added this to the v4.6 milestone Sep 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1875809: Restyle Time to inlineblock for Topology - Pipeline & Builds #6493

Bug 1875809: Restyle Time to inlineblock for Topology - Pipeline & Builds #6493

abhinandan13jan commented Sep 2, 2020

andrewballantyne Sep 3, 2020

andrewballantyne Sep 3, 2020

andrewballantyne Sep 3, 2020

andrewballantyne Sep 4, 2020

openshift-ci-robot commented Sep 4, 2020

abhinandan13jan commented Sep 4, 2020

abhinandan13jan commented Sep 4, 2020

andrewballantyne left a comment

abhinandan13jan commented Sep 8, 2020

andrewballantyne commented Sep 9, 2020

andrewballantyne commented Sep 9, 2020

christianvogt Sep 9, 2020 •

edited

abhinandan13jan Sep 10, 2020

christianvogt Sep 10, 2020

abhinandan13jan Sep 11, 2020

christianvogt Sep 11, 2020

abhinandan13jan Sep 11, 2020

christianvogt commented Sep 11, 2020

openshift-ci-robot commented Sep 11, 2020

openshift-ci-robot commented Sep 11, 2020

		@@ -31,7 +32,11 @@ const PipelineRunItem: React.FC<PipelineRunItemProps> = ({ pipelineRun }) => {
		<Grid hasGutter>

Bug 1875809: Restyle Time to inlineblock for Topology - Pipeline & Builds #6493

Bug 1875809: Restyle Time to inlineblock for Topology - Pipeline & Builds #6493

Conversation

abhinandan13jan commented Sep 2, 2020

Issue

Solution

Screenshot

Tests

Browser conformance

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

openshift-ci-robot commented Sep 4, 2020

abhinandan13jan commented Sep 4, 2020

abhinandan13jan commented Sep 4, 2020

andrewballantyne left a comment

Choose a reason for hiding this comment

abhinandan13jan commented Sep 8, 2020

andrewballantyne commented Sep 9, 2020

andrewballantyne commented Sep 9, 2020

christianvogt Sep 9, 2020 • edited

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 Sep 11, 2020

openshift-ci-robot commented Sep 11, 2020

openshift-ci-robot commented Sep 11, 2020

christianvogt Sep 9, 2020 •

edited