Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Art/96894] - poa search tabs #33602

Merged
merged 22 commits into from
Dec 31, 2024
Merged

[Art/96894] - poa search tabs #33602

merged 22 commits into from
Dec 31, 2024

Conversation

jquispe-oddball
Copy link
Contributor

Adding tabs to the POA Requests page, see existing ticket .

Merged in existing PR to get tabs to work with loaders.

Some notes:

  • pending tab state is the default state on page load
  • we are no longer showing a status badge for pending requests
Screen.Recording.2024-12-16.at.3.34.46.PM.mov

@jquispe-oddball jquispe-oddball requested review from a team as code owners December 16, 2024 20:53
Copy link

@va-vfs-bot va-vfs-bot left a comment

Choose a reason for hiding this comment

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

Icon found

Icons can be decorative, but sometimes they are used to convey meaning. If there are any semantics associated with an icon, those semantics should also be conveyed to a screen reader.

What you can do

Review the markup and see if the icon provides information that isn't represented textually, or wait for a VSP review.

Note:

Font Awesome is deprecated. Please use va-icon instead. For more information, visit the migration documentation: Migrate from font awesome to va-icon

{poaRequest.status === 'Pending' && (
<>
{expiresSoon(poaRequest.expiresAt) && (
<va-icon

Choose a reason for hiding this comment

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

icon found

@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 16, 2024 21:19 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 16, 2024 21:37 Inactive
@nihil2501 nihil2501 added the benefits-accredited-rep-facing Label for the OCTO Slack team #benefits-representative-facing label Dec 17, 2024
@nihil2501 nihil2501 marked this pull request as draft December 17, 2024 03:27
Copy link
Contributor

@nihil2501 nihil2501 left a comment

Choose a reason for hiding this comment

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

  • No results doesn't mean that tabs shouldn't render. They ought to be able to try a new query (change tab)
  • NavLink API aria-selected?
  • When building off another branch, branch off of it and open a PR into the branch you are branching off of: branch 1 <- branch 2

From the video, it seems like there are font regressions in multiple areas?

@jquispe-oddball jquispe-oddball force-pushed the art/96894/poa-search-tabs branch from d4b5ad4 to 6291cc3 Compare December 18, 2024 19:33
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 18, 2024 19:43 Inactive
@jquispe-oddball jquispe-oddball force-pushed the art/96894/poa-search-tabs branch from e5ff11b to 3e931a4 Compare December 20, 2024 12:36
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 20, 2024 20:14 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 20, 2024 23:45 Inactive
@jquispe-oddball jquispe-oddball force-pushed the art/96894/poa-search-tabs branch from 00846d3 to 08a41b4 Compare December 26, 2024 20:20
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 26, 2024 20:51 Inactive
@jquispe-oddball jquispe-oddball force-pushed the art/96894/poa-search-tabs branch from 03ecc2b to 6aa9833 Compare December 27, 2024 00:35
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 27, 2024 00:52 Inactive
@jquispe-oddball jquispe-oddball force-pushed the art/96894/poa-search-tabs branch from 7bf54af to 69cb04a Compare December 27, 2024 15:57
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 27, 2024 16:02 Inactive
@jquispe-oddball jquispe-oddball marked this pull request as ready for review December 27, 2024 18:15
@jquispe-oddball jquispe-oddball force-pushed the art/96894/poa-search-tabs branch from 69cb04a to 7d113ca Compare December 27, 2024 18:17
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 27, 2024 18:28 Inactive
Copy link

@va-vfs-bot va-vfs-bot left a comment

Choose a reason for hiding this comment

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

Icon found

Icons can be decorative, but sometimes they are used to convey meaning. If there are any semantics associated with an icon, those semantics should also be conveyed to a screen reader.

What you can do

Review the markup and see if the icon provides information that isn't represented textually, or wait for a VSP review.

Note:

Font Awesome is deprecated. Please use va-icon instead. For more information, visit the migration documentation: Migrate from font awesome to va-icon

{poaRequest.status === 'Pending' && (
<>
{expiresSoon(poaRequest.expiresAt) && (
<va-icon

Choose a reason for hiding this comment

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

icon found

@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 31, 2024 09:47 Inactive
Copy link

@va-vfs-bot va-vfs-bot left a comment

Choose a reason for hiding this comment

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

Icon found

Icons can be decorative, but sometimes they are used to convey meaning. If there are any semantics associated with an icon, those semantics should also be conveyed to a screen reader.

What you can do

Review the markup and see if the icon provides information that isn't represented textually, or wait for a VSP review.

Note:

Font Awesome is deprecated. Please use va-icon instead. For more information, visit the migration documentation: Migrate from font awesome to va-icon

{poaRequest.status === 'Pending' && (
<>
{expiresSoon(poaRequest.expiresAt) && (
<va-icon

Choose a reason for hiding this comment

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

icon found

@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 31, 2024 10:02 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/art/96894/poa-search-tabs/main December 31, 2024 11:58 Inactive
@nihil2501 nihil2501 requested a review from chumpy December 31, 2024 12:31
@chumpy chumpy merged commit d714d73 into main Dec 31, 2024
69 checks passed
@chumpy chumpy deleted the art/96894/poa-search-tabs branch December 31, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benefits-accredited-rep-facing Label for the OCTO Slack team #benefits-representative-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants