feat(chat message): add `author` and `timestamp` props by bmdalex · Pull Request #242 · microsoft/fluent-ui-react

bmdalex · 2018-09-17T18:32:03Z

Chat.Message: `author` and `timestamp`

This PR introduces author and timestamp props for Chat.Message subcomponent

TODO

`author`

Rendered only in "their" messages (mine=false).

<Chat.Message author="John Doe" content="Hello there"  />

renders

<li class="ui-layout ui-chat__message">
  <div class="ui-layout__main">
    <div class="ui-layout">
      <div class="ui-layout__start">
        <span class="ui-text a">John Doe</span>
      </div>
      <div class="ui-layout__main">Hello there</div>
    </div>
  </div>
</li>

`timestamp`

<Chat.Message timestamp="Yesterday, 10:15 PM" content="Hello there"  />

renders

<li class="ui-layout ui-chat__message">
  <div class="ui-layout__main">
    <div class="ui-layout">
      <div class="ui-layout__start">
        <span class="ui-text a">Yesterday, 10:15 PM</span>
      </div>
      <div class="ui-layout__main">Hello there</div>
    </div>
  </div>
</li>

kuzhelov · 2018-09-17T20:45:53Z

CHANGELOG.md

 ### Features
 - Add `FocusZone` to `renderComponent`, change `Menu` behavior to support arrow keys @tomasiser ([#116](https://github.com/stardust-ui/react/pull/116))
 - Add `value`, `disabled`, `checked`, `defaultChecked` and `onChange` props to `Radio` component @mnajdova ([#206](https://github.com/stardust-ui/react/pull/206))
+- Add `author` and `timestamp` props for `Chat.Message` component @Bugaa92 ([#242](https://github.com/stardust-ui/react/pull/242))


consider to move this entry to 'Unreleased' section

kuzhelov · 2018-09-17T20:54:22Z

src/components/Chat/ChatMessage.tsx

+      timestamp={timestamp}
+      content={content}
+      size="sm"
+      styles={{ root: { marginRight: pxToRem(10) } }}


we shouldn't hardcode styles in component's implementation code. Lets rather introduce timestamp part in the ChatMessage's styles, and provide this margin there

kuzhelov · 2018-09-17T20:58:27Z

src/components/Chat/ChatMessage.tsx

+
+  private renderText = (content: string, timestamp?: boolean) => (
+    <Text
+      timestamp={timestamp}


lets use two separate components for that - one for displaying timestamp, another one - to display content. This is necessary because Text component is seen to be a basic one, so it would support either timestamp variant, or regular content one. More than that, it is highly likely that timestamp variant won't be introduced as part of core Stardust

kuzhelov · 2018-09-17T20:59:27Z

src/themes/teams/components/Chat/chatMessageStyles.ts

 import { IChatMessageVariables } from './chatMessageVariables'
 import { pxToRem } from '../../../../lib'

+const rem10 = pxToRem(10)


would rather suggest name like px10asRem - otherwise the name is a bit misleading and reads like '10 rem'

kuzhelov · 2018-09-17T21:00:53Z

src/themes/teams/components/Chat/chatVariables.ts

@@ -0,0 +1,7 @@
+export interface IChatVariables {
+  backgroundColor: string


this is better to be handled by the code of examples for now - more than that, we could adjust padding aspects there as well

Actually, I added the backgroundColor and the padding to the variables with some defaults that would match the Team's theme, as we are developing this theme, so why not have this look by default. Please share your thoughts on this. Also, I added avatar slot in the ChatMessage, for configuring the statusBorderColor to match the background. Actually I have one thing that I am not 100% sure that should be implement this way. The Chat component has it's own backgroundColor and this color should match the Avatar's statusBorderColor. The problem is that, the Avatar is inside the ChatMessage component, and I am not confident that the Chat component should define the avatar's slot in the ChatMessage component. For now, those colors are independent variables in the Chat, and the ChatMessage's avatar slot component. What do you think about this @kuzhelov? This is how the default look of the examples looks now:

sounds quite reasonable, thanks!

agreed to not introduce these two as variables for now, just inline them into styles directly

We are going to stick with the backgroundColor as variable in order to reuse the siteVariables. Paddings are deleted from variables

mnajdova · 2018-09-18T10:15:12Z

src/components/Chat/ChatMessage.tsx

  mine?: boolean
  styles?: IComponentPartStylesInput
+  timestamp?: string
  variables?: ComponentVariablesInput


Why don't we make the text and the author shorthands for the Text component?

yeah - have discussed with @mnajdova, here proposal is essentially to introduce author and timestamp shorthands (slots) for the ChatMessage component, and use Text as a default component for these.

Totally support this idea 👍

mnajdova · 2018-09-18T10:16:27Z

Can we close #41 in favor of this one?

kuzhelov · 2018-09-18T12:24:53Z

sure, lets do that

codecov · 2018-09-19T11:34:24Z

Codecov Report

Merging #242 into master will increase coverage by 0.31%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
+ Coverage   91.84%   92.15%   +0.31%     
==========================================
  Files          61       61              
  Lines        1042     1058      +16     
  Branches      154      155       +1     
==========================================
+ Hits          957      975      +18     
+ Misses         82       80       -2     
  Partials        3        3

Impacted Files	Coverage Δ
src/components/Text/Text.tsx	`100% <100%> (ø)`	⬆️
src/components/Chat/ChatMessage.tsx	`97.5% <100%> (+1.5%)`	⬆️
src/components/Layout/Layout.tsx	`98% <0%> (+4%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5db6547...aa314f3. Read the comment docs.

kuzhelov · 2018-09-19T15:20:13Z

CHANGELOG.md

 ## [Unreleased]

+### BREAKING CHANGES
+- Fixed `Divider` wrong usage of the `typeSecondary{color, backgroundColor}` and `default{color, backgroundColor}` variables; renamed `default{color, backgroundColor}` variables to `color` and `backgroundColor` @mnajdova ([#234](https://github.com/stardust-ui/react/pull/234))


kuzhelov · 2018-09-19T15:21:08Z

src/components/Chat/ChatMessage.tsx


 export interface IChatMessageProps {
  as?: any
+  author?: ItemShorthand


# Conflicts: # CHANGELOG.md # src/components/Chat/ChatMessage.tsx

bmdalex requested review from dvdzkwsk, kuzhelov, levithomason, miroslavstastny and mnajdova as code owners September 17, 2018 18:32

feat(chat): add author and timestamp props

aa4ac4f

bmdalex force-pushed the feat/chat-author-and-timestamp branch from ba64493 to aa4ac4f Compare September 17, 2018 18:34

bmdalex added the 🚀 ready for review label Sep 17, 2018

kuzhelov reviewed Sep 17, 2018

View reviewed changes

kuzhelov added needs author changes Author needs to implement changes before merge and removed 🚀 ready for review labels Sep 17, 2018

mnajdova reviewed Sep 18, 2018

View reviewed changes

kuzhelov mentioned this pull request Sep 18, 2018

feat(Chat.Message): add 'author' and 'timestamp' props #41

Closed

11 tasks

manajdov added 2 commits September 19, 2018 11:31

Merge branch 'master' into feat/chat-author-and-timestamp

f9bbdb0

-addressing comments on the PR

ab2935e

-adding more variables to the chat and chat message components

8f463c4

mnajdova added 🚀 ready for review and removed needs author changes Author needs to implement changes before merge labels Sep 19, 2018

-removed timestamp right margin style

7210744

kuzhelov reviewed Sep 19, 2018

View reviewed changes

src/components/Chat/ChatMessage.tsx

export interface IChatMessageProps {

as?: any

author?: ItemShorthand

Copy link

Contributor

kuzhelov Sep 19, 2018

Choose a reason for hiding this comment

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

👍

kuzhelov approved these changes Sep 19, 2018

View reviewed changes

kuzhelov added ready for merge and removed 🚀 ready for review labels Sep 19, 2018

manajdov added 4 commits September 19, 2018 18:20

-removed padding variable

4aae38b

Merge branch 'master' into feat/chat-author-and-timestamp

fe4f871

# Conflicts: # CHANGELOG.md # src/components/Chat/ChatMessage.tsx

-removing root for the styles for the author and timestamp

a54c2e0

-rearranging imports to match master

aa314f3

mnajdova merged commit 98e70f2 into master Sep 19, 2018

bmdalex deleted the feat/chat-author-and-timestamp branch October 12, 2018 14:00

		@@ -0,0 +1,7 @@
		export interface IChatVariables {
		backgroundColor: string

Conversation

bmdalex commented Sep 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Chat.Message: author and timestamp

TODO

author

timestamp

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnajdova commented Sep 18, 2018

Uh oh!

kuzhelov commented Sep 18, 2018

Uh oh!

codecov bot commented Sep 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bmdalex commented Sep 17, 2018 •

edited

Loading

Chat.Message: `author` and `timestamp`

`author`

`timestamp`

codecov bot commented Sep 19, 2018 •

edited

Loading