-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat(web): Team List - Display email, phone and tags for team members #15947
feat(web): Team List - Display email, phone and tags for team members #15947
Conversation
WalkthroughThe pull request introduces updates to the team management components, enhancing the data structure and rendering of team member information. Key changes include the addition of new fields such as Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TeamList
participant GraphQL
participant TeamListSlice
User->>TeamList: Request team members
TeamList->>GraphQL: Execute GET_TEAM_MEMBERS_QUERY
GraphQL-->>TeamList: Return team members data (including email, phone, filterTags)
TeamList->>TeamListSlice: Render team members
TeamListSlice->>TeamListSlice: Process items with useMemo
TeamListSlice->>User: Display team members with details
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Additional context usedPath-based instructions (1)
Additional comments not posted (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Datadog ReportAll test runs ✅ 74 Total Test Services: 0 Failed, 72 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
apps/web/screens/queries/TeamList.ts (1)
17-28
: LGTM! The changes to theGET_TEAM_MEMBERS_QUERY
query are well-structured and aligned with the PR objective.The addition of new fields (
phone
, andfilterTags
) enhances the query's capability to retrieve more detailed information about team members. The structured approach used to define thefilterTags
object and its subfields adheres to GraphQL best practices.A few additional suggestions:
- Consider adding comments to describe the purpose of each new field, especially for the
filterTags
object and its subfields. This will improve the code readability and maintainability.- If the
phone
fields are optional, consider adding the@nullable
directive to indicate that they may return null values.- Verify that the corresponding changes have been made in the backend GraphQL schema to support these new fields.
Overall, the changes are well-implemented and do not introduce any breaking changes to the existing query structure.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- apps/web/components/Organization/Slice/TeamListSlice/TeamListSlice.tsx (4 hunks)
- apps/web/screens/queries/TeamList.ts (1 hunks)
- libs/cms/src/lib/generated/contentfulTypes.d.ts (1 hunks)
- libs/cms/src/lib/models/teamMember.model.ts (2 hunks)
- libs/island-ui/contentful/src/lib/TeamList/TeamList.css.ts (1 hunks)
- libs/island-ui/contentful/src/lib/TeamList/TeamList.tsx (4 hunks)
Additional context used
Path-based instructions (6)
libs/island-ui/contentful/src/lib/TeamList/TeamList.css.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/web/screens/queries/TeamList.ts (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/cms/src/lib/models/teamMember.model.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/web/components/Organization/Slice/TeamListSlice/TeamListSlice.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/island-ui/contentful/src/lib/TeamList/TeamList.tsx (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/cms/src/lib/generated/contentfulTypes.d.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
Biome
libs/island-ui/contentful/src/lib/TeamList/TeamList.tsx
[error] 195-195: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
Additional comments not posted (15)
libs/island-ui/contentful/src/lib/TeamList/TeamList.css.ts (1)
7-9
: LGTM!The new
style
function from@vanilla-extract/css
aligns with the reusability and TypeScript guidelines for thelibs
directory.libs/cms/src/lib/models/teamMember.model.ts (3)
16-20
: LGTM!The addition of the optional
phone
fields to theTeamMember
class is a valid enhancement to the data model. The usage of the@Field
decorator withnullable: true
is correct for optional fields. The changes adhere to TypeScript usage for defining props.
42-43
: LGTM!The update to the
mapTeamMember
function to include the newphone
fields is consistent with the changes made to theTeamMember
class. The data transformation logic is correctly modified to accommodate the new fields.
Line range hint
1-44
: Adheres to the additional instructions.The code in this file adheres to the following:
- Reusability of components and hooks across different NextJS apps: The
TeamMember
model class is reusable across different NextJS apps.- TypeScript usage for defining props and exporting types: TypeScript is used for defining the class properties and decorators.
- Effective tree-shaking and bundling practices: The file exports the
TeamMember
class and themapTeamMember
function, which can be effectively tree-shaken and bundled.apps/web/components/Organization/Slice/TeamListSlice/TeamListSlice.tsx (3)
14-14
: LGTM!The code change is approved.
70-106
: Great work on optimizing theitems
computation and improving the tag grouping logic!
- The use of
useMemo
is a good optimization to avoid unnecessary re-computations of theitems
array.- The logic for creating
tagGroups
is correctly implemented:
- It checks for valid tags before processing them.
- It dynamically updates the
tagGroups
array by either pushing to existing groups or creating new ones.- It ensures that group labels end with a colon.
- The sorting of
tagGroups
usingsortAlpha
improves the organization and readability of the rendered output.The code changes are approved.
135-140
: Nice addition of theprefixes
prop for better localization!The
prefixes
object conditionally sets the labels for email and phone based on the active locale, allowing for better localization of the displayed information in theTeamList
component.The code changes are approved.
libs/island-ui/contentful/src/lib/TeamList/TeamList.tsx (7)
21-21
: LGTM!The addition of the optional
variant
property to theTeamListProps
interface is a good way to support different display modes for the team list component.
22-25
: LGTM!The addition of the optional
prefixes
property to theTeamListProps
interface is a good way to provide customizable labels for the email and phone fields.
34-34
: LGTM!The addition of the optional
teamMembers
structure within theTeamListProps
interface is a good way to store the email address of a team member.
35-35
: LGTM!The addition of the optional
phone
property to theteamMembers
structure within theTeamListProps
interface is a good way to store the phone number of a team member.
36-39
: LGTM!The addition of the optional
tagGroups
property to theteamMembers
structure within theTeamListProps
interface is a good way to categorize team members using tags.
143-144
: LGTM!The modification of the
TeamMemberAccordionList
component's props to include theprefixes
property is consistent with the addition of theprefixes
property to theTeamListProps
interface and allows for passing customizable labels for the email and phone fields to the component.
Line range hint
1-221
: Code adheres to the additional instructions.The code follows the guidelines mentioned in the additional instructions:
The
TeamList
component and its sub-components are reusable across different NextJS apps as they are self-contained and rely only on the props passed to them.TypeScript is used for defining the
TeamListProps
interface and the props of theTeamList
component and its sub-components, ensuring type safety and better developer experience.The code uses named exports for the
TeamList
component and its sub-components, allowing for effective tree-shaking by bundlers to reduce the bundle size.Tools
Biome
[error] 195-195: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
libs/cms/src/lib/generated/contentfulTypes.d.ts (1)
4411-4417
: LGTM!The addition of the optional
phone
, andintro
properties to theITeamMemberFields
interface is approved. These changes expand the data structure to include more detailed information about team members.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UI Core files look good
…#15947) * Add explicit email and phone fields to team member content type * Display filter tags * Add missing key prop * Sort imports --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Team List - Display email, phone and tags for team members
What
Why
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
email
,phone
, andtagGroups
.