-
Notifications
You must be signed in to change notification settings - Fork 71
[LG-5645] feat(chat-layout): implement ChatMain component #3252
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
Conversation
|
6948c53 to
d1812aa
Compare
|
Size Change: +250 B (+0.02%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
663fa6c to
ae7a9f9
Compare
d1812aa to
b7d4136
Compare
b7d4136 to
5e4db61
Compare
| expect(screen.getByText('Main Content')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('forwards HTML attributes to the div element', () => { |
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.
Do we need to test this?
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.
Yes, I'm planning to include these in tests from now on to test if HTML attributes are settable. In some cases, our components intentionally don't offer this. It felt worth testing to more explicitly document that ComponentPropsWith*Ref is part of the type interface, and it's a low effort inclusion
| expect(ref.current?.tagName).toBe('DIV'); | ||
| }); | ||
|
|
||
| test('applies custom className', () => { |
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.
Same for this test. Do we need to test this?
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.
Yes, I'm planning to test this moving forward as well. It helps document to make sure className is properly passed through to the root node, and it's also a low effort inclusion
|
|
||
| export type ChatMainProps = ComponentPropsWithRef<'div'> & | ||
| DarkModeProps & | ||
| PropsWithChildren<{}>; |
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.
Should this be required?
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.
I think we'll also need to omit children from ComponentPropsWithRef
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.
Dropped this in ChatLayout and ChatMain. For some reason, google + the imported type shows that ComponentProps* doesn't include children, but I guess it actually does
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.
Looks good! 🌻 Just a few nits
|
Coverage after merging steph/chat-layout-main into LG-5439/chat-layout will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* feat(chat-layout): implement ChatMain component * refactor(chat-layout): remove PropsWithChildren
* feat(chat-layout): implement ChatMain component * refactor(chat-layout): remove PropsWithChildren
* feat(chat-layout): implement ChatMain component * refactor(chat-layout): remove PropsWithChildren
✍️ Proposed changes
This PR introduces the
ChatMaincomponent to the@lg-chat/chat-layoutpackage, which represents the main content area of the chat layout. The component automatically positions itself in the CSS Grid system and provides a structured container for chat content.Key changes:
ChatMaincomponent with proper TypeScript types, styles, and comprehensive testsChatLayout.stories.tsxwith a complete working example that includes TitleBar, ChatWindow, MessageFeed, Message, and InputBar components@lg-chat/leafygreen-chat-provideras a peer dependency@lg-chat/title-bar,@lg-chat/chat-window,@lg-chat/message-feed,@lg-chat/message,@lg-chat/input-bar) as dev dependencies🎟️ Jira ticket: LG-5645
✅ Checklist
pnpm changesetand documented my changes🧪 How to test changes
Composition/Chat/ChatLayoutpnpm test chat-layoutto verify all ChatMain tests pass