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

[MINT-3043] Front End work to create Documents Card #1342

Merged
merged 39 commits into from
Sep 9, 2024

Conversation

garyjzhao
Copy link
Contributor

@garyjzhao garyjzhao commented Sep 4, 2024

MINT-3043

Description

  • Created DocumentsCard component and its corresponding files
  • Add copy to collaborationArea.ts
  • Add fileType to modelPlan query
  • Updated documents.spec.js cypress test
  • Moved shared styles from ModelPlanCard/index.scss and DocmentsCard/index.scss into the index.scss in parent folder
  • Removed DocumentBanner from Tasklist

How to test this change

  1. Navigate to model collaboration area and verify that Documents card is there
  2. Click button to add document
  3. Go through the steps to add a link
  4. Return to the model collaboration area and verify that it now says "1 link added"
  5. Go through the steps to add another link and verify that it now says "2 links added"
  6. Go through the steps to add a PDF and verify that it now says "1 uploaded"
  7. Verify that View all button shows up only when there is something added/uploaded
  8. Verify that breadcrumbs are updated
    NOTE: If accessing "Add Documents" directly from the collaboration area, hide the Documents breadcrumb

PR Author Checklist

  • I have provided a detailed description of the changes in this PR.
  • I have provided clear instructions on how to test the changes in this PR.
  • I have updated tests or written new tests as appropriate in this PR.
  • Updated the Postman Collection if necessary.

PR Reviewer Guidelines

  • It's best to pull the branch locally and test it, rather than just looking at the code online!
  • When approving a PR, provide a reason why you're approving it
    • e.g. "Approving because I tested it locally and all functionality works as expected"
    • e.g. "Approving because the change is simple and matches the Figma design"
  • Don't be afraid to leave comments or ask questions, especially if you don't understand why something was done! (This is often a great time to suggest code comments or documentation updates)
  • Check that all code is adequately covered by tests - if it isn't feel free to suggest the addition of tests.

@garyjzhao garyjzhao marked this pull request as ready for review September 4, 2024 19:08
@garyjzhao garyjzhao requested a review from a team as a code owner September 4, 2024 19:08
@garyjzhao garyjzhao requested review from patrickseguraoddball and removed request for a team September 4, 2024 19:08
Copy link
Contributor

@patrickseguraoddball patrickseguraoddball left a comment

Choose a reason for hiding this comment

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

I think we are going to have conflicts once mine is merged, just noting them here

cypress/e2e/documents.spec.js Outdated Show resolved Hide resolved
src/views/ModelPlan/CollaborationArea/index.scss Outdated Show resolved Hide resolved
src/views/ModelPlan/CollaborationArea/index.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@patrickseguraoddball patrickseguraoddball left a comment

Choose a reason for hiding this comment

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

Spacing here is a bit tight compared to Figma

Screenshot 2024-09-05 at 9 01 36 AM

I think we can probably update the new breadcrumb text here for IT solutions tracker. This can be done directly in the breadcrumb component. We also want to add the Model Plan task list breadcrumb here until we build out the collab area card for tracker. This was just fixed in figma

Screenshot 2024-09-05 at 9 07 03 AM

Copy link
Contributor

@patrickseguraoddball patrickseguraoddball 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, I'll await an approval until QA is done testing in dev

Copy link
Contributor

@patrickseguraoddball patrickseguraoddball left a comment

Choose a reason for hiding this comment

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

lg

@garyjzhao garyjzhao merged commit 48cb339 into main Sep 9, 2024
28 checks passed
@garyjzhao garyjzhao deleted the MINT-3043/FE-documents-card branch September 9, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants