Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web: add red border to failing steps #4164

Merged
merged 3 commits into from Aug 1, 2019
Merged

web: add red border to failing steps #4164

merged 3 commits into from Aug 1, 2019

Conversation

jomsie
Copy link

@jomsie jomsie commented Jul 25, 2019

fixes #4111

Release Note

\note{feature}{
  We noticed after \link{PR #4058}{https://github.com/concourse/concourse/pull/4058} 
  (where build steps are collapsed by default) that it wasn't very easy to see failing steps.
  
  Now a failed step has a simple red border around its header and an erroring step has an
  orange border
  \link{#4164}{https://github.com/concourse/concourse/pull/4164} 
}

#4111

Signed-off-by: James Thomson <jthomson@pivotal.io>
web/elm/src/Build/Styles.elm Outdated Show resolved Hide resolved
no shifting when a step fails

#4111

Signed-off-by: Jamie Klassen <cklassen@pivotal.io>
Co-authored-by: James Thomson <jthomson@pivotal.io>
@jamieklassen jamieklassen requested a review from vito July 31, 2019 15:01
StepStateFailed ->
Colors.failure

_ ->
Copy link
Member

@vito vito Jul 31, 2019

Choose a reason for hiding this comment

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

👻 Scope creeping, but can we do the same for errored steps? Those are in the same boat: they cause a build failure (well, error) but are hard to notice when auto-collapsed.

I don't think there are any other states we care about. 🤔

Also maybe we can just explicitly write out all the states here so the compiler will help us find places to update if/when we add another state.

Copy link
Member

Choose a reason for hiding this comment

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

Here's a prototype, but let's get @Lindsayauchin in on this sooner rather than later.
Screen Shot 2019-07-31 at 3 21 45 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

@vito @pivotal-jamie-klassen Makes sense. I like it!

Copy link
Member

Choose a reason for hiding this comment

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

@Lindsayauchin @pivotal-jamie-klassen More 👻 scope creep: what do you think of adding a yellow border for running steps, since they're auto-collapsed too?

some context: #2608 (comment)

#4111

Signed-off-by: James Thomson <jthomson@pivotal.io>
Co-authored-by: Jamie Klassen <cklassen@pivotal.io>
Copy link
Member

@vito vito left a comment

Choose a reason for hiding this comment

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

💯

@jomsie
Copy link
Author

jomsie commented Jul 31, 2019

wats test failing, running locally it sometimes passes when it doesn't timeout

@jamieklassen jamieklassen merged commit c2233dc into master Aug 1, 2019
@jamieklassen jamieklassen deleted the issue/4111 branch August 1, 2019 14:32
@cirocosta
Copy link
Member

hey, I just wanted to say that this is amazing ❤️ ! thank you all! (@xtremerui shares the same thought!)

@jomsie jomsie added the release/documented Documentation and release notes have been updated. label Aug 26, 2019
jamieklassen pushed a commit that referenced this pull request Aug 26, 2019
#4164
#4250

Signed-off-by: James Thomson <jthomson@pivotal.io>
Co-authored-by: Jamie Klassen <cklassen@pivotal.io>
matthewpereira pushed a commit that referenced this pull request Sep 5, 2019
#4164
#4250

Signed-off-by: James Thomson <jthomson@pivotal.io>
Co-authored-by: Jamie Klassen <cklassen@pivotal.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/documented Documentation and release notes have been updated. web-ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make auto-collapsed failed steps easier to notice
5 participants