cloud-gov / cg-ui

for the 2024 18F-supported cloud.gov product UI formerly known as the Stratos Dashboard
Other
4 stars 0 forks source link

Functional users list overlays #483

Closed echappen closed 1 month ago

echappen commented 1 month ago

Blocked by PR #476

Changes proposed in this pull request:

Per engineering sync discussion, some additional acceptance criteria:

How to test

  1. Log into the CloudFoundry dev environment through the CLI: cf login -a [url] --sso
  2. Start up a dev server with a refreshed CF token: npm run dev-cf
  3. Navigate to the users list: /orgs/[orgId]
  4. For the org roles form, click any link under the "organization roles" column
  5. For the space roles form, click any link under the "access permissions" column

Forms should operate the same as when they were on pages, with the only difference being that the overlay closes on form submission success.

Related issues

Closes #468

Submitter checklist

Security considerations

None, UI changes only

echappen commented 1 month ago

@beepdotgov @cannandev @hursey013 The only criteria I'm still having trouble with is announcing the success message—can't get it to announce. I'll keep working on that, but otherwise, I think this is ready for eyes!

echappen commented 1 month ago

I'm going to make dynamic url params a separate GH issue—I might get to it depending on how much I need to update with this PR, but I don't want it to block the rest of the work.

echappen commented 1 month ago

I’ve been working on getting aria-live regions to be more consistently read by VoiceOver. The lack of consistency still perplexes me.

I know that in order for aria-live regions to be read, two things need to be true (this article is a good summary):

  1. aria-live regions need to be present in the DOM on initial page load, and
  2. Screen readers will only read updates to these regions (it won’t read text already present on page load)

I suspect the inconsistency is coming down to the timing between a react component re-rendering (thus re-loading that part of the DOM) and a message updating. I think the reason that the success message wasn’t being read was because these things were happening at the same time.

Putting a 1/2 second delay on the message update solved this issue. But I’ve also been looking into alternatives, and I’m intrigued by this approach, which is now an npm package. This approach uses the React Context API, which I haven’t used at all, so I’m less comfortable with it. @cannandev @hursey013 Any thoughts on this approach and if we should try it?