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

feature/8942-ClaimFilesSegment #9066

Merged
merged 23 commits into from
Jul 29, 2024
Merged

Conversation

Sparowhawk
Copy link
Contributor

@Sparowhawk Sparowhawk commented Jul 12, 2024

Description of Change

Created Files layout and segmented controller title change from details to files. Also created unit tests. This is behind the feature flag. See slack thread for VQA

Screenshots/Video

Screenshot 2024-07-12 at 11 04 17 AM Screenshot 2024-07-18 at 11 11 06 AM

Testing

yarn test

  • Tested on iOS
  • Tested on Android

Reviewer Validations

Use a row component to represent each file associated with claim

Within the row component, display these additional fields associated with each file:

Filename
Request type
Document type
Received
VQA Misty / Lauren

PR Checklist

Reviewer: Confirm the items below as you review

  • PR is connected to issue(s)
  • Tests are included to cover this change (when possible)
  • No magic strings (All string unions follow the Union -> Constant type pattern)
  • No secrets or API keys are checked in
  • All imports are absolute (no relative imports)
  • New functions and Redux work have proper TSDoc annotations

For QA

Run a build for this branch

@Sparowhawk Sparowhawk requested review from a team as code owners July 12, 2024 16:05
@Sparowhawk Sparowhawk changed the title files finished no unit tests yet, and no null handling for request ty… feature/8942-ClaimFilesSegment Jul 12, 2024
@Sparowhawk Sparowhawk marked this pull request as draft July 12, 2024 16:06
@Sparowhawk Sparowhawk marked this pull request as ready for review July 12, 2024 16:56
alexandec
alexandec previously approved these changes Jul 15, 2024
@github-actions github-actions bot added FE-With QA A PR waiting for QA Activity and removed FE-Needs Review labels Jul 15, 2024
@rbontrager rbontrager removed FE-With QA A PR waiting for QA Activity labels Jul 17, 2024
@dumathane dumathane added the FE-Changes Requested Requested changes from Eng or QA label Jul 18, 2024
@github-actions github-actions bot added the FE-With QA A PR waiting for QA Activity label Jul 18, 2024
alexandec
alexandec previously approved these changes Jul 22, 2024
alexandec
alexandec previously approved these changes Jul 23, 2024
Copy link
Contributor

@alexandec alexandec left a comment

Choose a reason for hiding this comment

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

Approving with optional suggestions

@@ -16,23 +16,41 @@ function ClaimFiles({ claim }: ClaimFilesProps) {
const { t } = useTranslation(NAMESPACE.COMMON)
const theme = useTheme()
const { attributes } = claim
const documents = attributes.eventsTimeline.filter((event) => event.filename && event.filename.length > 0)
const events = attributes.eventsTimeline.filter(
(event) => (event.filename && event.filename.length > 0) || (event.documents && event.documents.length > 0),
Copy link
Contributor

Choose a reason for hiding this comment

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

Optionally could simplify these checks like

(event.filename?.length || event.documents?.length)

if (document.filename) {
const textLines: TextLine[] = [{ text: document.filename, variant: 'MobileBodyBold' }]
if (document.fileType) {
textLines.push({ text: t('appointmentList.requestType', { type: document.fileType }) })
Copy link
Contributor

Choose a reason for hiding this comment

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

Optionally could simplify all these to one-liners like

document.fileType && textLines.push({ text: t('appointmentList.requestType', { type: document.fileType }) })

if (document.uploadDate) {
textLines.push({ text: t('appointmentList.received', { date: document.uploadDate }) })
}
items.push({ textLines: textLines })
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use shorthand here

@Sparowhawk Sparowhawk requested a review from alexandec July 24, 2024 16:40
alexandec
alexandec previously approved these changes Jul 24, 2024
rbontrager
rbontrager previously approved these changes Jul 29, 2024
@github-actions github-actions bot added FE-Ready to Merge and removed FE-With QA A PR waiting for QA Activity labels Jul 29, 2024
@dumathane dumathane dismissed stale reviews from rbontrager and alexandec via 3ea00b9 July 29, 2024 19:15
@dumathane dumathane merged commit 681d9c9 into develop Jul 29, 2024
18 of 21 checks passed
@dumathane dumathane deleted the feature/8942-ClaimFilesSegment branch July 29, 2024 19:23
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.

4 participants