Skip to content

Conversation

@laurenolivia
Copy link
Contributor

@laurenolivia laurenolivia commented Apr 18, 2025

Description

This PR replaces instances of <input.label> with Hds::Form::Label, and those instances only exist on our role/grants form.

Update:
Usage of input.field cause a11y violations, so we agreed to expand the scope of this ticket to also refactor the input field.

🎟️ Jira ticket

Screenshots (if appropriate)

Before 👇
Screenshot 2025-04-18 at 12 43 13 PM

After 👇
Screenshot 2025-04-29 at 2 45 20 PM

How to Test

Checklist

  • I have added before and after screenshots for UI changes
  • I have added JSON response output for API changes
  • I have added steps to reproduce and test for bug fixes in the description
  • I have commented on my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

@laurenolivia laurenolivia requested a review from a team as a code owner April 18, 2025 16:43
@vercel
Copy link

vercel bot commented Apr 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
boundary-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 2, 2025 2:57pm
boundary-ui-desktop ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 2, 2025 2:57pm

@laurenolivia laurenolivia changed the title replace with hds::form::label Replace with Hds::Form::Label Apr 18, 2025
@laurenolivia laurenolivia force-pushed the laurenolivia/replace-rose-form-label branch from 1c6abbf to a01ada3 Compare April 18, 2025 16:57
@laurenolivia laurenolivia marked this pull request as draft April 18, 2025 17:20
Copy link
Collaborator

@hashicc hashicc left a comment

Choose a reason for hiding this comment

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

With migrating just the label (Rose::Form::Label) and not the input (Rose::Form::Input) that yields the label (input.label) it looks like the a11y wiring between the label and the input is broken: a quick test for this is to click the label, Grant or New Grant and it should focus on the input. Instead of trying to restore the label linkage it might make more sense to migrate the Rose::Form::Input to Hds::Form::Input.

@laurenolivia
Copy link
Contributor Author

With migrating just the label (Rose::Form::Label) and not the input (Rose::Form::Input) that yields the label (input.label) it looks like the a11y wiring between the label and the input is broken: a quick test for this is to click the label, Grant or New Grant and it should focus on the input. Instead of trying to restore the label linkage it might make more sense to migrate the Rose::Form::Input to Hds::Form::Input.

@hashicc thanks for the feedback!

cc: @DhariniJeeva - do you mind if we increase the scope of this ticket?

@laurenolivia
Copy link
Contributor Author

With migrating just the label (Rose::Form::Label) and not the input (Rose::Form::Input) that yields the label (input.label) it looks like the a11y wiring between the label and the input is broken: a quick test for this is to click the label, Grant or New Grant and it should focus on the input. Instead of trying to restore the label linkage it might make more sense to migrate the Rose::Form::Input to Hds::Form::Input.

@hashicc thanks for the feedback!

cc: @DhariniJeeva - do you mind if we increase the scope of this ticket?

Oh, actually, I just checked our epics. It doesn't appear that we have a ticket for replacing Rose::Form::Input (we do have a ticket to retire the component, though), so if it's ok with you, I will just make that change within this PR and update the ticket to expand the scope?

@DhariniJeeva
Copy link
Collaborator

With migrating just the label (Rose::Form::Label) and not the input (Rose::Form::Input) that yields the label (input.label) it looks like the a11y wiring between the label and the input is broken: a quick test for this is to click the label, Grant or New Grant and it should focus on the input. Instead of trying to restore the label linkage it might make more sense to migrate the Rose::Form::Input to Hds::Form::Input.

@hashicc thanks for the feedback!
cc: @DhariniJeeva - do you mind if we increase the scope of this ticket?

Oh, actually, I just checked our epics. It doesn't appear that we have a ticket for replacing Rose::Form::Input (we do have a ticket to retire the component, though), so if it's ok with you, I will just make that change within this PR and update the ticket to expand the scope?

that's a good point, I'm totally ok with migrating the input in this PR

@laurenolivia laurenolivia changed the title Replace with Hds::Form::Label Replace with Hds::Form::Label and Input Field Apr 28, 2025
@laurenolivia laurenolivia force-pushed the laurenolivia/replace-rose-form-label branch from f4759d8 to ec748ba Compare April 28, 2025 15:51
Copy link
Collaborator

@ZedLi ZedLi 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!

Copy link
Collaborator

@priya-patel04 priya-patel04 left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes!

@laurenolivia laurenolivia force-pushed the laurenolivia/replace-rose-form-label branch from a2d0978 to e24e129 Compare May 2, 2025 14:54
@laurenolivia laurenolivia merged commit dd8e186 into main May 2, 2025
13 checks passed
@laurenolivia laurenolivia deleted the laurenolivia/replace-rose-form-label branch May 2, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants