DEVPROD-10197: Implement broken versions warning on waterfall #438
Conversation
| @@ -18,6 +19,11 @@ export const InactiveVersionsButton: React.FC<Props> = ({ | |||
| containerHeight, | |||
| versions, | |||
| }) => { | |||
| const numBrokenVersions = | |||
There was a problem hiding this comment.
[super nit] We more commonly name our variables like brokenVersionsCount and I personally prefer that
| @@ -46,7 +46,12 @@ export const WaterfallGrid: React.FC<WaterfallGridProps> = ({ | |||
| version ? ( | |||
| <VersionLabel key={version.id} size="small" {...version} /> | |||
| ) : ( | |||
There was a problem hiding this comment.
If you calculate the inactive version count here, then you can pass a value into each component (either the count or hasError) instead of having to both count and use the .some() function in two separate places
There was a problem hiding this comment.
I think the current code is a little cleaner. Since versions.inactiveVersions is an array, I would have to associate the versionCount with the fist versionId in the list and pass it through to BuildRow (I don't like the id association bit). Alternatively, I could update the typing for the inactiveVersions and include a field that flags if a broken version exists but I actually like how the the component accepts the exact GQL typing.
|
evergreen retry |
| @@ -4,7 +4,8 @@ import { wordBreakCss } from "components/styles"; | |||
| import { size } from "constants/tokens"; | |||
|
|
|||
| const BUILD_VARIANT_WIDTH = 200; | |||
| const INACTIVE_WIDTH = 80; | |||
| const INACTIVE_WITHOUT_ERROR_WIDTH = 80; | |||
| const INACTIVE_WITH_ERROR_WIDTH = 160; | |||
There was a problem hiding this comment.
As mentioned above
| const INACTIVE_WITH_ERROR_WIDTH = 160; | |
| const INACTIVE_WITH_ERROR_WIDTH = 150; |
| @@ -63,3 +75,8 @@ const InactiveVersionLine = styled.div<{ containerHeight: number }>` | |||
| const StyledVersionLabel = styled(VersionLabel)` | |||
| padding-top: ${size.xs}; | |||
| `; | |||
|
|
|||
| const StyledBadge = styled(Badge)` | |||
| top: -${size.xs}; | |||
There was a problem hiding this comment.
[nit] I know the designs have this slightly above the rest of the elements but I think positioning all the filters will be easier if it's inline with the top of VersionLabel with some padding between it and the button
Co-authored-by: Sophie Stadler <[email protected]>
| {brokenVersionsCount > 0 && ( | ||
| <StyledBadge data-cy="broken-versions-badge" variant={Variant.Red}> | ||
| {brokenVersionsCount} broken{" "} | ||
| {pluralize("version", brokenVersionsCount)} |
There was a problem hiding this comment.
Sorry for not suggesting this earlier, but what do you think of omitting "versions" entirely (i.e. "1 Broken")? I don't think the phrasing is so helpful that it warrants taking away space from the useful columns on the page. Without it we could just have one fixed width for the InactiveVersion div I think.
DEVPROD-10197
Description
Shows a badge on the waterfall and inactive/unmatching versions modal.
Screenshots
Evergreen PR
evergreen-ci/evergreen#8374