search

readthedocs / ext-theme

Read the Docs drop in replacement site templates
2 stars 2 forks source link

Build details page: match colors for cancelled builds #269

Closed humitos closed 8 months ago

humitos commented 10 months ago

I like that you used a different color (🟧 ) for commands that output with the custom exit code.

Screenshot_2024-01-10_16-59-26

Now, the build is not exactly failed. Would it make sense to mark is as cancelled instead? as we are doing in https://beta.readthedocs.org/projects/dev/builds/23063631/ for example

Screenshot_2024-01-10_17-01-49

(opened from a conversation in Slack)

It seems the logic to distinguish this is implemented in the front-end because the backend does not return what's expected:

https://github.com/readthedocs/ext-theme/blob/a39cec9063a82d55e1a549f914d14c90dbb2e506/src/js/build/detail.js#L249-L301

humitos commented 10 months ago

I think I found the issue.

The new notification system does not use populate Build.error anymore, so this check is not accurate here since this.error() will always be empty: https://github.com/readthedocs/ext-theme/blob/178a684b281c221f0fd608cf3e44fa1f32130fff/src/js/build/detail.js#L273

agjohnson commented 8 months ago

I believe this is similar to the backend bug at:

With that bug fixed, the UI looks correct to me now when cancelling builds manually:

image

Would it make sense to mark is as cancelled instead?

Yeah exactly. For this UI to show builds cancelled by the 183 exit code, the backend should set the build state to cancelled. The front end should already have the correct logic to support this state, but if not we can reopen this.

I opened this to address the backend change: