Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat(chat): add avatar prop to chat message subcomponent #159

Merged
merged 2 commits into from
Sep 4, 2018

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Aug 29, 2018

Chat

This PR add the avatar prop to the Chat.Message subcomponent. This provides the ability to add an avatar next to a chat message and specify the whole spectrum of properties an avatar can have.

TODO

  • Conformance test
  • Minimal doc site example
  • Stardust base theme
  • Teams Light theme
  • Teams Dark theme
  • Teams Contrast theme
  • Confirm RTL usage
  • W3 accessibility check
  • Stardust accessibility check
  • Update glossary props table
  • Update the CHANGELOG.md

API Proposal

avatar

screen shot 2018-08-29 at 18 34 28

avatar is a shorthand for the Avatar component; it can be:

  • a string that would give the avatar's name;
  • a React Element encapsulating an Avatar component;
  • an object depicting Avatar component's props:
  alt?: string
  as?: any
  className?: string
  name?: string
  size?: number
  src?: string
  status?:
    | 'Available'
    | 'Away'
    | 'BeRightBack'
    | 'Busy'
    | 'DoNotDisturb'
    | 'Offline'
    | 'PresenceUnknown'
  getInitials?: (name: string) => string
  styles?: IComponentPartStylesInput
  variables?: ComponentVariablesInput
<Chat>
  <Chat.Message mine avatar={{ src: 'matt.jpg', status: 'Available' }}>
    Hello
  </Chat.Message>

  <Chat.Message avatar={{ src: 'jenny.jpg', status: 'Busy' }}>
    Hi
  </Chat.Message>

  <Chat.Message mine avatar={{ src: 'matt.jpg', status: 'Available' }}>
    Let's go get some lunch!
  </Chat.Message>

  <Chat.Message avatar={{ src: 'jenny.jpg', status: 'Busy' }}>
    Sure thing. I was thinking we should try the new place downtown.
  </Chat.Message>
</Chat>

generates:

<ul>
  <li>
    <div>Hello</div>
    <div>
      <img src="matt.jpg" />
      <div>
        <span title="Available"></span>
      </div>
    </div>
  </li>
  <li>
    <div>
      <img src="jenny.jpg" />
      <div>
        <span title="Busy"></span>
      </div>
    </div>
    <div>Hi</div>
  </li>
  <li>
    <div>Let&#x27;s go get some lunch!</div>
    <div>
      <img src="matt.jpg" />
      <div>
        <span title="Available"></span>
      </div>
    </div>
  </li>
  <li>
    <div>
      <img src="jenny.jpg" />
      <div>
        <span title="Busy"></span>
      </div>
    </div>
    <div>Sure thing. I was thinking we should try the new place downtown.</div>
  </li>
</ul>

@codecov
Copy link

codecov bot commented Aug 29, 2018

Codecov Report

Merging #159 into master will decrease coverage by 0.15%.
The diff coverage is 60.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
- Coverage   68.02%   67.87%   -0.16%     
==========================================
  Files         101      101              
  Lines        1370     1385      +15     
  Branches      269      274       +5     
==========================================
+ Hits          932      940       +8     
- Misses        436      442       +6     
- Partials        2        3       +1
Impacted Files Coverage Δ
src/components/Avatar/Avatar.tsx 91.66% <100%> (+0.23%) ⬆️
...emes/teams/components/Chat/chatMessageVariables.ts 100% <100%> (ø) ⬆️
.../themes/teams/components/Chat/chatMessageStyles.ts 23.07% <11.11%> (-10.26%) ⬇️
src/components/Chat/ChatMessage.tsx 95.65% <90.9%> (-4.35%) ⬇️

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 a26e33b...83d0977. Read the comment docs.

src/components/Chat/ChatMessage.tsx Outdated Show resolved Hide resolved
marginTop: '1rem',
marginBottom: '1rem',
...(props.mine
...(p.mine
Copy link
Member

Choose a reason for hiding this comment

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

Levi always prefers destructuring objects at the beginning of a function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well it was not destructured before and doing so will increase code size; Levi prefers shorter code as well; I think destructuring should happen once we use more props there or logic becomes more complex

src/components/Chat/ChatMessage.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@kuzhelov kuzhelov left a comment

Choose a reason for hiding this comment

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

the important thing that blocks PR from being merged is necessity to use shorthand factory for creating child Avatar component. As an example of that, please, consider how icon property is introduced for Input component

@kuzhelov kuzhelov added needs author changes Author needs to implement changes before merge and removed 🚀 ready for review labels Aug 30, 2018
@bmdalex bmdalex force-pushed the feat/chat-avatar branch 4 times, most recently from 72624f7 to 043ed7d Compare September 3, 2018 17:29
@bmdalex
Copy link
Collaborator Author

bmdalex commented Sep 3, 2018

@kuzhelov , @miroslavstastny - thanks for reviewing, addressed all comments

@bmdalex bmdalex added 🚀 ready for review and removed needs author changes Author needs to implement changes before merge labels Sep 3, 2018
@bmdalex bmdalex force-pushed the feat/chat-avatar branch 3 times, most recently from 1d946b8 to 3c1fc7b Compare September 4, 2018 12:43
@bmdalex bmdalex merged commit cd54229 into master Sep 4, 2018
@bmdalex bmdalex deleted the feat/chat-avatar branch September 7, 2018 14:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants