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

Bold in-progress diffs diffrently #7918

Merged
merged 10 commits into from
Sep 28, 2021
Merged

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Sep 7, 2021

Description

This feature makes it easier to see which diffs are in progress vs completed. It does this by bolding the colors of steps that are in progress.

Fixes #7337

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

@iwahbe iwahbe self-assigned this Sep 13, 2021
@susanev
Copy link
Contributor

susanev commented Sep 15, 2021

@iwahbe thanks for chatting with me today!!

here's my recommendation: lets leave all colors alone except for the +, -, and status. and because of the accessibility issues here lets use the colors you have for created and deleted (and corresponding +,-), and have creating and deleting (and corresponding +,-) be the same white as the other text in the table. folks will be able to more easily scan the green/white and red/white vs green/light-green and red/light-red.

what do you think?

@iwahbe
Copy link
Member Author

iwahbe commented Sep 16, 2021

I think that folks will be more able to scan the green/neutral and red/neutral then the color mix. The problem I have is that except at stack creation and stack destruction, most commands will invoke a combination of create, delete, update and replace CRUD operations, each represented with their own color. My example didn't have that because I'm not playing with a complicated stack. If we implement your suggestion, we will get create visibility into what is in progress vs done, but will loose visibility into what type of operations are in progress.

Right now I have it set up to split the difference. The user can choose if they want to see full color (difference between progress and pending within a color), or partial color (only completed operations are colored).

@susanev How does that look from your end?

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 8685e25

@github-actions
Copy link

Diff for pulumi-random with merge commit 8685e25

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 8685e25

@github-actions
Copy link

Diff for pulumi-aws with merge commit 8685e25

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 8685e25

@github-actions
Copy link

Diff for pulumi-azure with merge commit 8685e25

@susanev
Copy link
Contributor

susanev commented Sep 16, 2021

lets def decide on one change together. i dont htink this situation calls for allowing folks to pick two different solutions. imma take a closer look at all the colors we are using so we can figure out a way to go with your solution but also not introduce contrast issues. ill get back to ya as soon as i can.

@iwahbe iwahbe removed the request for review from leezen September 27, 2021 18:32
@iwahbe iwahbe changed the title Color in-progress diffs diffrently Bold in-progress diffs diffrently Sep 28, 2021
@susanev
Copy link
Contributor

susanev commented Sep 28, 2021

im not gonna approve cause i havent reviewed the code, but based on the video you sent me im good with this change. thx for doing this work!!

Copy link
Member

@komalali komalali left a comment

Choose a reason for hiding this comment

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

This looks great @iwahbe ! Just a couple of minor clean-up questions and then should be good to go.

CHANGELOG_PENDING.md Outdated Show resolved Hide resolved
pkg/backend/display/diff.go Outdated Show resolved Hide resolved
pkg/backend/display/progress.go Outdated Show resolved Hide resolved
pkg/backend/display/rows.go Outdated Show resolved Hide resolved
pkg/cmd/pulumi/pulumi.go Outdated Show resolved Hide resolved
pkg/engine/diff.go Outdated Show resolved Hide resolved
sdk/go/common/diag/colors/colors.go Outdated Show resolved Hide resolved
@iwahbe
Copy link
Member Author

iwahbe commented Sep 28, 2021

@komalali Thanks for pointing out the convention.

Is this a leftover?

This was a good catch!

Curious why you removed the colors. from the comment here?

Because it made less sense in an earlier commit. I should have restored it. (and now I did)

@justinvp
Copy link
Member

I haven't had a chance to try this out. Mind sharing the video, or a screenshot, or https://asciinema.org?

@iwahbe
Copy link
Member Author

iwahbe commented Sep 28, 2021

I haven't had a chance to try this out. Mind sharing the video, or a screenshot, or https://asciinema.org?

I'd be happy to.
https://user-images.githubusercontent.com/22222529/135171528-b71759bf-cfc5-4a6e-84fc-b00425aad17a.mov

Copy link
Member

@komalali komalali left a comment

Choose a reason for hiding this comment

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

LGTM!

@iwahbe iwahbe merged commit 9df8d3a into master Sep 28, 2021
@pulumi-bot pulumi-bot deleted the iwahbe/7337/diff-in-progress-colors branch September 28, 2021 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Distinguish complete (created) from in-progress (`creating) tasks
4 participants