Submission workflow UX improvement of CI checks#4377
Submission workflow UX improvement of CI checks#4377richardhjtan wants to merge 1 commit intomainfrom
Conversation
| } | ||
|
|
||
| export class CiSection extends GlimmerComponent<CiSectionSignature> { | ||
| get flatItems() { |
There was a problem hiding this comment.
perhaps you should use @cached so we don't rebuild this everytime this.flatItems is consumed, and only if ciGroups actually changes
There was a problem hiding this comment.
Pull request overview
Improves the CI checks UX in the submission/PR workflow by avoiding the “empty state ↔ running state” flicker during live query refreshes.
Changes:
- Add
isLoading-aware UI states for CI checks (PR card CI section + CI status field + submission workflow step detail). - Stabilize CI checks rendering by sorting CI items alphabetically and adding stable
{{#each ... key=...}}keys. - Thread
ciIsLoadingthrough relevant components/state resolution to drive the new loading messaging.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/catalog-realm/submission-workflow-card/submission-workflow-card.gts | Adds ciIsLoading into workflow state resolution and stabilizes steps rendering with keyed each. |
| packages/catalog-realm/pr-card/utils.ts | Stabilizes CI item ordering and adjusts status text for in-progress checks. |
| packages/catalog-realm/pr-card/pr-card.gts | Computes/passes CI loading state into the CI section. |
| packages/catalog-realm/pr-card/fields/ci-status-field.gts | Adds a dedicated loading row UI when CI data is still loading. |
| packages/catalog-realm/pr-card/components/isolated/ci-section.gts | Adds loading state rendering and flattens CI groups for stable keyed iteration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{! ── Step tracker ── }} | ||
| <div class='sw-steps'> | ||
| {{#each this.workflowState.steps as |step idx|}} | ||
| {{#each this.workflowState.steps key="key" as |step idx|}} |
There was a problem hiding this comment.
idx from {{#each ... as |step idx|}} is a number, but this.lastStepIndex is defined as a string (String(this.workflowState.steps.length - 1)). Since eq uses strict equality, the "last step" branch will never match and the connector will still render for the last step. Consider making lastStepIndex a number (or casting idx to string) so the comparison works as intended.
| : state === 'in_progress' | ||
| ? 'In Progress' | ||
| : formatCiValue(status); |
There was a problem hiding this comment.
This change collapses all non-completed check statuses (e.g. GitHub's queued / requested) into the display label "In Progress" whenever state === 'in_progress' and conclusion is null. If you want to preserve the more specific status text, consider only using the "In Progress" fallback when status is missing, otherwise keep formatCiValue(status).
| : state === 'in_progress' | |
| ? 'In Progress' | |
| : formatCiValue(status); | |
| : status != null | |
| ? formatCiValue(status) | |
| : state === 'in_progress' | |
| ? 'In Progress' | |
| : formatCiValue(status); |
| <div class='ci-item loading-state'> | ||
| <CiDot @state='in_progress' /> | ||
| <div class='ci-item-detail'> | ||
| <span class='ci-item-name loading-text'>Loading CI checks...</span> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
The loading state renders a standalone <div class='ci-item ...'> instead of keeping the same list semantics used for actual items (<ul role='list'><li ...>). For more consistent accessibility and less layout/semantics churn, consider rendering a <ul> in the loading state as well (with a single <li>), or adding an explicit role="status" / aria-live to the loading container.
| <div class='ci-item loading-state'> | |
| <CiDot @state='in_progress' /> | |
| <div class='ci-item-detail'> | |
| <span class='ci-item-name loading-text'>Loading CI checks...</span> | |
| </div> | |
| </div> | |
| <ul class='ci-group' role='list'> | |
| <li class='ci-item loading-state'> | |
| <CiDot @state='in_progress' /> | |
| <div class='ci-item-detail'> | |
| <span class='ci-item-name loading-text'>Loading CI checks...</span> | |
| </div> | |
| </li> | |
| </ul> |
This is to fix the flickering issue on the CI checks section, where it flashes back and forth from the empty state to the pipelines running state when the query gets new cards.
Before
Screen.Recording.2026-04-09.at.9.44.50.PM.mov
After
Screen.Recording.2026-04-10.at.1.18.08.PM.mov