Skip to content

fix(#3657): use the goa progress component in the temp notification#3831

Open
chrisolsen wants to merge 1 commit intodevfrom
chris/3657-temp-notification-progress-bar
Open

fix(#3657): use the goa progress component in the temp notification#3831
chrisolsen wants to merge 1 commit intodevfrom
chris/3657-temp-notification-progress-bar

Conversation

@chrisolsen
Copy link
Copy Markdown
Collaborator

Before (the change)

image

After (the change)

image

Make sure that you've checked the boxes below before you submit the PR

  • I have read and followed the setup steps
  • I have created necessary unit tests
  • I have tested the functionality in both React and Angular.

Steps needed to test

Can use the existing prs feature to test this out http://localhost:xxxx/features/2730

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 16, 2026

Deploy Preview for benji-docs-previews failed.

Name Link
🔨 Latest commit 2e15444
🔍 Latest deploy log https://app.netlify.com/projects/benji-docs-previews/deploys/69ea61d36f9d5800087cb8e5

twjeffery
twjeffery previously approved these changes Apr 17, 2026
Copy link
Copy Markdown
Collaborator

@twjeffery twjeffery left a comment

Choose a reason for hiding this comment

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

@chrisolsen, my PR #3806 touches the same file with some visual changes (shadow, min-heights, action button, mobile wrapping). I'll rebase on top of yours once it merges and handle the goa-linear-progress color and size overrides there so we're not stepping on each other.

@chrisolsen chrisolsen force-pushed the chris/3657-temp-notification-progress-bar branch from 69383ff to 4208468 Compare April 20, 2026 15:56
@twjeffery twjeffery requested a review from ArakTaiRoth April 20, 2026 23:41
@chrisolsen chrisolsen force-pushed the chris/3657-temp-notification-progress-bar branch from 4208468 to ea6bbd1 Compare April 21, 2026 14:42
Copy link
Copy Markdown
Collaborator

@vanessatran-ddi vanessatran-ddi left a comment

Choose a reason for hiding this comment

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

Looks good!

@ArakTaiRoth ArakTaiRoth linked an issue Apr 22, 2026 that may be closed by this pull request

{#if type === "progress"}
<progress data-testid="progress" value={progress} max="100" />
<goa-linear-progress progress={progress} max="100" percent-visibility="hidden" />
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There's no ariaLabel provided for this, so it fails the accessibility criteria

<goa-linear-progress progress={progress} max="100" percent-visibility="hidden" />
{:else if type === "indeterminate"}
<progress />
<goa-linear-progress percent-visibility="hidden" />
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There's no ariaLabel provided for this, so it fails the accessibility criteria


{#if type === "progress"}
<progress data-testid="progress" value={progress} max="100" />
<goa-linear-progress progress={progress} max="100" percent-visibility="hidden" />
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is no max property

@ArakTaiRoth ArakTaiRoth force-pushed the chris/3657-temp-notification-progress-bar branch from ea6bbd1 to 2e15444 Compare April 23, 2026 18:15
@ArakTaiRoth ArakTaiRoth dismissed stale reviews from vanessatran-ddi and twjeffery April 23, 2026 18:17

Changes were made

Copy link
Copy Markdown
Collaborator

@vanessatran-ddi vanessatran-ddi left a comment

Choose a reason for hiding this comment

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

Looks good!

@ArakTaiRoth ArakTaiRoth requested a review from twjeffery April 27, 2026 18:00
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.

Add linear progress to temporary notification

4 participants