-
Notifications
You must be signed in to change notification settings - Fork 310
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: add chat icons #2789
feat: add chat icons #2789
Conversation
adds icons for group chat and meeting chats fixes overall layout of chat header refactors ChatHeader in to smaller components adds a css property for aligning items in the mgt-person fixes padding around chat messages and sendbox
Thank you for creating a Pull Request @gavinbarron. This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:
|
🚀 New react-contoso sample application deployed here |
🚀 New react-contoso sample application deployed here |
📖 The updated storybook is available here |
🚀 New react-contoso sample application deployed here |
1 similar comment
🚀 New react-contoso sample application deployed here |
|
Thanks for the review. Oh, that's subtle and I didn't see it! good catch!
Ahh, because they're not buttons anymore, I'll fix this CSS.
It's using the brand colors provided by the fluent provider. At present that's hardcoded to be
The alignment is wildly inconsistent across the designs, some have the roster management button and the avatar/icon left aligned, some have them with icon for the roster management center aligned with the avatar/icon. |
📖 The updated storybook is available here |
🚀 New react-contoso sample application deployed here |
@sebastienlevert in the interests of moving this forward I suggest we take theming and padding as separate work items to ensure we don't get stalled on getting this set of improvement merged. This PR is already more complex than I hoped it would be thanks to refactoring a number of components #2216 already exists for theming and needs definition. Do we need a separate work here to cover dark/light toggling? |
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.
Good to go.
📖 The updated storybook is available here |
🚀 New react-contoso sample application deployed here |
packages/mgt-chat/src/components/ChatHeader/GroupChatHeader.tsx
Outdated
Show resolved
Hide resolved
6de8d58
packages/mgt-chat/src/components/ChatHeader/GroupChatHeader.tsx
Outdated
Show resolved
Hide resolved
📖 The updated storybook is available here |
🚀 New react-contoso sample application deployed here |
📖 The updated storybook is available here |
I have made the all the alignment to left and made padding to 20px on both short and wide form in the Figma file. Please let me know this helped. |
Closes #2788
PR Type
Description of the changes
adds icons for group chat and meeting chats
fixes overall layout of chat header
refactors ChatHeader in to smaller components
adds a css property for aligning items in the mgt-person fixes padding around chat messages and sendbox
PR checklist
yarn build
) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)yarn setLicense
)Other information