feat(standups): Responses grid with static prompt #6353

BartoszJarocki · 2022-04-06T11:52:28Z

For now, we'll use the same grid as we use on the meeting dashboard. In next iterations we'll improve and implement #6389.
Earlier discussion for the reference: #5965

This PR also adds #6200 as it was a little change.

…en team member

BartoszJarocki · 2022-04-13T07:44:06Z

@jmtaber129 would you mind taking a look? thanks!

jmtaber129 · 2022-04-14T13:29:34Z

packages/client/components/TeamPromptMeeting.tsx

+  const teamMembers = useMemo(() => {
+    return meeting.phases
+      .flatMap((phase) => phase.stages)
+      .flatMap((stage) => stage.teamMember)


+1 Just mapping to stages should be a bit more representative, since the response cards encapsulate the full stage with both team member and response, but this is fine as-is for the purposes of implementing the grid

yeah, I'm pretty sure it'll need to be adjusted to the card component anyway so I didn't want to guess. if you don't mind, I'll leave it as is

jmtaber129 · 2022-04-14T13:53:59Z

packages/client/components/TeamPromptMeeting.tsx

+  padding: '0 8px'
+})
+
+const TeamMemmberName = styled('h3')({


+1 TeamMemmberName -> TeamMemberName

jmtaber129 · 2022-04-14T13:56:07Z

packages/client/components/TeamPromptMeeting.tsx

@@ -22,23 +86,80 @@ const TeamPromptMeeting = (props: Props) => {
    graphql`
      fragment TeamPromptMeeting_meeting on TeamPromptMeeting {
        ...TeamPromptTopBar_meeting
+        id


+1 This id doesn't seem to be used anywhere

jmtaber129 · 2022-04-14T13:57:29Z

packages/client/components/TeamPromptMeeting.tsx

+          phaseType
+          stages {
+            ... on TeamPromptResponseStage {
+              id


+1 This id doesn't seem to be used anywhere (this seems like a reasonable key for response cards, but we're currently using teamMember.id)

jmtaber129 · 2022-04-14T14:06:28Z

packages/client/components/TeamPromptMeeting.tsx

+
+                      return (
+                        <TeamMemberResponse
+                          key={id}


+1 Would it make sense to use child.key here? That would guarantee consistency between the key used by useTransition and the key used by React.

for sure. technically, it's the same thing but I agree, it's cleaner

jmtaber129 · 2022-04-14T14:08:24Z

packages/client/components/TeamPromptMeeting.tsx

+          id
+          phaseType


+1 id and phaseType don't seem to be used anywhere

BartoszJarocki · 2022-04-15T13:16:42Z

@jmtaber129 I fixed every comment except for the mapping which I think should be aligned in the PR related to cards. thank you for the review, moving on to the maintainer review 🙏

BartoszJarocki · 2022-04-18T19:45:59Z

@mattkrick I'd be great if you could take a look 🙏 thank you!

mattkrick

some very minor client-side stuff, after that feel free to merge!

mattkrick · 2022-04-19T01:24:57Z

packages/client/components/TeamPromptMeeting.tsx

@@ -22,23 +86,76 @@ const TeamPromptMeeting = (props: Props) => {
    graphql`
      fragment TeamPromptMeeting_meeting on TeamPromptMeeting {
        ...TeamPromptTopBar_meeting
+        ...useMeeting_meeting
+        phases {
+          stages {


-1 fragment on TeamPromptResponsesPhase instead. it's a little more expressive way to say "we only care about this 1 phase in particular"

as it's written, it's saying, "look at EVERY phase & for EVERY phase, give me the TeamPromptResponsesStages"

Also, if you fragment on the phase, then the teamMember object is guaranteed non null, which is nice.

oh cool, will fix

mattkrick · 2022-04-19T01:33:34Z

packages/client/components/TeamPromptMeeting.tsx

+        ...useMeeting_meeting
+        phases {
+          stages {
+            ... on TeamPromptResponseStage {


-1 Could you please rename TeamPromptResponsesPhase to TeamPromptResponsePhase? (notice the missing "s"). not a problem in this PR, but i didn't discover it until now

no strong opinions here, the origin of the name is the fact that this phase is for all responses, not one. let me know if that makes sense. if you think it's still better to use TeamPromptResponsePhase I will fix

ah gotcha, that works!

mattkrick · 2022-04-19T01:36:12Z

packages/client/components/TeamPromptMeeting.tsx

+        }
+      })
+      .filter(isNotNull)
+  }, [meeting.phases])


-1 with the changes above, you can simplify this to:

const responsePhase = meeting.phases.find((phase) => phase.__typename === 'TeamPromptResponsePhase')! const teamMembers responsePhase.stages.map((stage) => {...stage.teamMember, key: stage.teamMember.id}))

@mattkrick typescript is a little dumb in that case and responsePhase can be another type as well.
see the screenshot below

do we already have some generic utility to generate type predicates? we'd need something like this and I just want to make sure I haven't missed anything in the codebase
https://stackoverflow.com/questions/59050071/narrow-return-type-of-find-from-discriminated-union-array

ah good call, we have GQLType, which does the trick. I'll write a little commit to showcase it

mattkrick · 2022-04-21T22:28:00Z

packages/client/components/TeamPromptMeeting.tsx

-      })
-      .filter(isNotNull)
-  }, [meeting.phases])
+  const phase = getPhaseByTypename(phases, 'TeamPromptResponsesPhase')


@BartoszJarocki let me know if this gets you whatcha need. if so, go ahead & merge this PR & we'll start using this more in the future

yup, looks good! thank you!

* fix: not add another responses stage if stage already exist for a given team member * feat: added responses phase to phaseTypeToSlug lookup * feat: added team member to team prompt response stage gql type * feat: team prompt grid * feat: added static team prompt question * code cleanup * add getPhaeByTypename Co-authored-by: Matt Krick <matt.krick@gmail.com>

BartoszJarocki force-pushed the feat/6201/standups-grid branch 2 times, most recently from bbbeb63 to eb76aa0 Compare April 7, 2022 13:57

BartoszJarocki added 3 commits April 8, 2022 08:11

fix: not add another responses stage if stage already exist for a giv…

65c62b8

…en team member

feat: added responses phase to phaseTypeToSlug lookup

9d32e87

feat: added team member to team prompt response stage gql type

bf1b5d9

BartoszJarocki force-pushed the feat/6201/standups-grid branch from eb76aa0 to 66f77e0 Compare April 8, 2022 06:11

BartoszJarocki linked an issue Apr 8, 2022 that may be closed by this pull request

Standup UI - Responses grid #6201

Closed

1 task

feat: team prompt grid

11ddf26

BartoszJarocki force-pushed the feat/6201/standups-grid branch from 66f77e0 to 11ddf26 Compare April 12, 2022 15:10

feat: added static team prompt question

d82f0f3

jmtaber129 mentioned this pull request Apr 12, 2022

feat(standups): Response Cards #6392

Merged

BartoszJarocki marked this pull request as ready for review April 13, 2022 07:43

BartoszJarocki requested a review from jmtaber129 April 13, 2022 07:43

BartoszJarocki changed the title ~~feat(standups): Responses grid~~ feat(standups): Responses grid with static prompt Apr 13, 2022

BartoszJarocki linked an issue Apr 13, 2022 that may be closed by this pull request

Standup UI - Static, non-editable prompt #6200

Closed

2 tasks

jmtaber129 approved these changes Apr 14, 2022

View reviewed changes

BartoszJarocki added 2 commits April 15, 2022 15:06

Merge branch 'master' into feat/6201/standups-grid

ca33424

code cleanup

a50910a

BartoszJarocki requested a review from mattkrick April 18, 2022 19:45

mattkrick requested changes Apr 19, 2022

View reviewed changes

add getPhaeByTypename

a4f49ea

mattkrick reviewed Apr 21, 2022

View reviewed changes

BartoszJarocki merged commit 244cec2 into master Apr 22, 2022

BartoszJarocki deleted the feat/6201/standups-grid branch April 22, 2022 06:57

github-actions bot mentioned this pull request Sep 26, 2022

chore: release v6.75.1 #7229

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(standups): Responses grid with static prompt #6353

feat(standups): Responses grid with static prompt #6353

BartoszJarocki commented Apr 6, 2022 •

edited

Loading

BartoszJarocki commented Apr 13, 2022

jmtaber129 Apr 14, 2022

BartoszJarocki Apr 15, 2022

jmtaber129 Apr 14, 2022

BartoszJarocki Apr 15, 2022

jmtaber129 Apr 14, 2022

jmtaber129 Apr 14, 2022

jmtaber129 Apr 14, 2022

BartoszJarocki Apr 15, 2022

jmtaber129 Apr 14, 2022

BartoszJarocki commented Apr 15, 2022

BartoszJarocki commented Apr 18, 2022

mattkrick left a comment

mattkrick Apr 19, 2022

BartoszJarocki Apr 19, 2022

mattkrick Apr 19, 2022

BartoszJarocki Apr 19, 2022

mattkrick Apr 21, 2022

mattkrick Apr 19, 2022

BartoszJarocki Apr 19, 2022 •

edited

Loading

mattkrick Apr 21, 2022

mattkrick Apr 21, 2022

BartoszJarocki Apr 22, 2022

feat(standups): Responses grid with static prompt #6353

feat(standups): Responses grid with static prompt #6353

Conversation

BartoszJarocki commented Apr 6, 2022 • edited Loading

BartoszJarocki commented Apr 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BartoszJarocki commented Apr 15, 2022

BartoszJarocki commented Apr 18, 2022

mattkrick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BartoszJarocki Apr 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BartoszJarocki commented Apr 6, 2022 •

edited

Loading

BartoszJarocki Apr 19, 2022 •

edited

Loading