Skip to content

feat: Redesign build logs #4734

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

Merged
merged 9 commits into from
Oct 25, 2022
Merged

feat: Redesign build logs #4734

merged 9 commits into from
Oct 25, 2022

Conversation

BrunoQuaresma
Copy link
Collaborator

Before:
Screen Shot 2022-10-24 at 17 00 12

After:
Screen Shot 2022-10-24 at 17 00 24

@BrunoQuaresma BrunoQuaresma requested a review from a team as a code owner October 24, 2022 20:02
@BrunoQuaresma BrunoQuaresma requested review from Kira-Pilot and removed request for a team October 24, 2022 20:02
@BrunoQuaresma BrunoQuaresma self-assigned this Oct 24, 2022
Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

The code and design look great to me!

Should we add stories for the build table to make changes a bit simpler?

@BrunoQuaresma
Copy link
Collaborator Author

BrunoQuaresma commented Oct 24, 2022

@kylecarbs the build table already has stories. But let me check how good is its coverage.

spacing={1}
>
<span>
<strong>{initiatedBy}</strong>{" "}
Copy link
Member

Choose a reason for hiding this comment

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

We can add translations with formatting by using a Trans component.
Presley asked me to write out an example, so there's one in our how-tos.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found it hard to use Trans here because of the conditionals but I moved the text to translation files.

<TableRow
hover
data-testid={`build-${build.id}`}
className={styles.buildRow}
Copy link
Member

Choose a reason for hiding this comment

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

This is such a nice, tabbable table! I would love to be able to hit 'enter' and navigate to the log page without having to touch my mouse.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was watching TV when I "remembered" that I forgot to add the link 😆 I'm glad you found it as well, really good review 🌮

@Kira-Pilot
Copy link
Member

I cannot actually click into the logs anymore. Is this by design? If we've decided not to show the logs anymore, can we remove the pointer and table highlight on mouseover?

@BrunoQuaresma BrunoQuaresma merged commit 3e08bb4 into main Oct 25, 2022
@BrunoQuaresma BrunoQuaresma deleted the bq/improve-build-log branch October 25, 2022 03:44
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants