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.

Sign up for GitHub

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

Jump to bottom

VACMS-16432 banner updates #294

Merged
merged 35 commits into from
Dec 29, 2023
Merged

VACMS-16432 banner updates #294

merged 35 commits into from
Dec 29, 2023

Conversation

tjheffner
Copy link
Contributor

Description

Relates to #16432. (or closes?)

  • Updates banner query & getGlobalElements to be more consistent with how other data is queried and formatted.
  • Fixed some unused variable warnings in the playwright a11y test and staticProps

Testing done

  • created a facility banner locally and saw that it rendered in the new approach

Screenshots

QA steps

What needs to be checked to prove this works?
What needs to be checked to prove it didn't break any related things?
What variations of circumstances (users, actions, values) need to be checked?

Tasks
Preview Give feedback

Is this PR blocked by another PR?

  • Add the DO NOT MERGE label
  • Add links to additional PRs here:

tjheffner and others added 15 commits December 14, 2023 10:01
@tjheffner
fix unused imports
75eb04c
@tjheffner
move banner data fetch to query
7c9d531
@tjheffner
move auth check to preview fetch only
679b8fc
@tjheffner
banners mostly refactored
bd789a7
@tjheffner
dont remove path
016d9eb
@olivereri @tjheffner
Verify VA Edge Devices Support Secure TLS Renegotiation (#282)
ac44864
@tjheffner
Redis hotfix (#283)
1a663c8
@dependabot @tjheffner
Bump @storybook/addon-interactions from 7.6.4 to 7.6.5 (#286)
4d6fdad 
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @tjheffner
Bump @axe-core/playwright from 4.8.1 to 4.8.2 (#288)
4c11701 
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @tjheffner
Bump @types/node from 20.4.4 to 20.10.5 (#289)
19aa7d2 
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @tjheffner
Bump @typescript-eslint/eslint-plugin from 6.8.0 to 6.14.0 (#290)
e58090e 
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @tjheffner
Bump actions/upload-artifact from 3 to 4 (#291)
3468dcf 
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@tjheffner
move auth check to preview fetch only
4209e88
@tjheffner
keep global auth for now
7d462f9
@tjheffner
const to let
71b102a
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 18, 2023 19:09 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 18, 2023 19:15 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 18, 2023 20:19 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 20, 2023 00:27 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 20, 2023 21:19 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 20, 2023 21:21 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 20, 2023 21:29 Destroyed
@tjheffner tjheffner requested review from ryguyk and jtmst December 20, 2023 21:49
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 26, 2023 17:26 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 27, 2023 20:28 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 28, 2023 20:54 Destroyed
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 28, 2023 20:55 Destroyed
ryguyk
ryguyk reviewed Dec 28, 2023
View reviewed changes
Copy link
Contributor

@ryguyk ryguyk left a comment

Choose a reason for hiding this comment

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

Just a few comments. The changes look good overall. I think my biggest question is around the location of the type definitions.

next.config.js Outdated Show resolved Hide resolved
src/types/dataTypes/formatted/banners.ts Outdated Show resolved Hide resolved
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 28, 2023 22:31 Destroyed
@tjheffner tjheffner requested a review from ryguyk December 28, 2023 22:59
@va-cms-bot va-cms-bot temporarily deployed to Tugboat December 28, 2023 23:34 Destroyed
@tjheffner tjheffner enabled auto-merge (squash) December 28, 2023 23:41
@tjheffner tjheffner merged commit c670c93 into main Dec 29, 2023
5 of 6 checks passed
@tjheffner tjheffner deleted the VACMS-16432-banner-updates branch December 29, 2023 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryguyk ryguyk ryguyk approved these changes

@jtmst jtmst Awaiting requested review from jtmst

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tjheffner @ryguyk @va-cms-bot @olivereri