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

[EASI-4639] Discussions panel views #2895

Merged

Conversation

aterstriep
Copy link
Contributor

@aterstriep aterstriep commented Nov 20, 2024

EASI-4639

Figma

Description

  • Components to handle different views within GRB Discussions side panel
  • ViewDiscussions - Internal GRB discussion board view with list of discussions
    • src/views/DiscussionBoard/ViewDiscussions.tsx
  • StartDiscussion - Start discussion form
    • src/views/DiscussionBoard/StartDiscussion.tsx
  • Discussion - View and reply to discussion, displays list of replies
    • src/views/DiscussionBoard/Discussion.tsx

To do:

  • Follow-up PR:
    • Add MentionTextArea to form and DiscussionCard
    • Unit tests for form
    • Refetch discussions after form submit
      -This causes bug where side panel rerenders after refetch
  • Fix navigation between panels in EASI-4667
    • Navigation from buttons
    • Navigation after form submit

How to test this change

  • In GRB review admin view, click "View discussion board" button to view side panel
  • Additional view components are commented out here. Manually switch out active component for testing.
  • Unit tests handle additional view cases:
    • src/views/DiscussionBoard/ViewDiscussions.test.tsx
    • src/views/DiscussionBoard/Discussion.test.tsx

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.

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.

@aterstriep aterstriep changed the base branch from main to feature/EASI-4614_grb_discussions November 20, 2024 00:10
@aterstriep aterstriep marked this pull request as ready for review November 20, 2024 15:32
@aterstriep aterstriep requested a review from a team as a code owner November 20, 2024 15:32
@aterstriep aterstriep requested review from adamodd and removed request for a team November 20, 2024 15:32
@adamodd adamodd self-assigned this Nov 20, 2024
@adamodd adamodd mentioned this pull request Nov 20, 2024
Copy link
Contributor

@adamodd adamodd left a comment

Choose a reason for hiding this comment

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

Nevermind, good bit of updates are in #2896

There are some repeating 00000000-0000-0000-0000-000000000000 grbDiscussions element ids causing dupe key warnings on the FE.

@adamodd adamodd self-requested a review November 20, 2024 21:35
@adamodd adamodd dismissed their stale review November 20, 2024 21:36

Old find

* MentionTextArea in DiscussionForm

* Mention area height-auto

* Text

* Rhf-like signature

* .usa-textarea + overrides

* Cleanup

* MentionTextArea React.forwardRef

* Cleanup
@ClayBenson94
Copy link
Collaborator

ClayBenson94 commented Nov 22, 2024

@aterstriep I think I get some errors (Page not found) when loading requests that don't have any discussions. I think that's something we'd chatted about before, and just wasn't sure if it's something you're tracking for this PR or a future one, but just wanted to make sure!

EDIT: I think we're all set, I see what's going on!
Guessing it's this code that's causing it.

// Get the first discussion from the array for testing purposes
const activeDiscussion = grbDiscussions.length > 0 ? grbDiscussions[0] : null;

I assume once we have navigation hooked up we'll probably either pass discussion post IDs directly when navigating to them or just navigate straight to <ViewDiscussions>, which doesn't seem to have this issue 😁

@aterstriep aterstriep merged commit 820a81d into feature/EASI-4614_grb_discussions Nov 22, 2024
12 checks passed
@aterstriep aterstriep deleted the EASI-4639/discussions-panel-views branch November 22, 2024 15:48
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.

3 participants