feat(chat): chat item gutter and refactoring #556

bmdalex · 2018-12-04T18:34:33Z

feat(chat): chat item gutter and refactoring

Description

This PR implements solution 1 from #537 and closes #537

We need to refactor our Chat subcomponents, Chat.Item and Chat.Message for the following reasons:

need better structuring and more flexibility;
need to solve specific clients needs when it comes to layout of components and providing very specific content to specific areas;
Chat.Message is too specific (the avatar should be more top level)
Chat.Item is not specific enough (needs gutter)

Solution (implemented in this PR)

This PR implements a mix of solution 1 from #537 and other discussion I had with the team; here are the main steps:

removing avatar slot from Chat.Message
adding gutter slot to Chat.Item;
adding gutterPosition='start' | 'end' prop to Chat.Item;
renaming content slot to message for Chat.Item;
deprecating Children API for Chat components;

API:

<Chat items={[
  {
    message: <AnyComponent .../>,
  },
  {
    mine: true,
    gutterPosition: 'end',
    gutter: { content: <Avatar image="" status="" /> },
    message: { content:
      <Chat.Message
        content="Let's have a call"
        author="John Doe"
        timestamp="Today, 11:15 PM"
        mine
      />,
    },
  },
]} />

Known issues:

need to add mine prop to Chat.Item as well for styling purposes and deciding on gutter positioning; other suggestion was: gutterPosition: 'before' | 'after' (*LE: decided gutterPosition: 'start' | 'end')
in order to leverage correct styles for gutter and content slots we will need to specify the content and gutter props as objects with content props instead of just elements after merging fix(factories): shorthand fix for applying props to react element #496 (*LE: we will use verbose content props for now and look into RFC: Shorthand polymorphism using kind property #512 to simplify that)
chatItemStyles are too specific and are causing other components to be rendered incorrectly as content (Divider for instance, see below); we might have to go with solution 2 from Chat.Item and Chat.Mesasge refactor #537 and create subcomponents. (*LE: fixed by using new styles that do not rely on flexbox, by using css children selectors for styling)
maxWidth: 80% for gutter + content requires complex styling in order to be achieved (*LE: fixed by using new styles that do not rely on flexbox, by using css children selectors for styling)

*LE: ALL ISSUES ARE HANDLED, GOOD FOR REVIEW

Legend:

*LE: later edit

src/components/Chat/ChatItem.tsx

src/components/Chat/ChatMessage.tsx

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

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

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

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

mnajdova · 2018-12-05T11:30:14Z

Some thoughts on the raised issues:

need to add mine prop to Chat.Item as well for styling purposes and deciding on gutterpositioning; other suggestion was: gutterPosition: 'before' | 'after'

This was my only proposal on this, the positioning is something general that we have in multiple components, so it would be great if we are consistent about this.

in order to leverage correct styles for gutter and content slots we will need to specify the content and gutter props as objects with content props instead of just elements after merging #496

For this, I don't have smarter proposal (currently this is the only way we can make 'generic' slots. After implementing the 'kind' property, see issue: #512 we may provide different API here)

chatItemStyles are too specific and are causing other components to be rendered incorrectly as content (Divider for instance, see below); we might have to go with solution 2 from #537 and create subcomponents.

I would suggest to always use one display prop for component and try to arrange the items there dependently on the props. If we are using flex, then we should always stick with it. One simple example of how it can be refactored to use just flex is the following (dirty code alert - take this just as a example):

const chatItemStyles: ComponentSlotStylesInput<ChatItemProps, any> = {
  root: ({ props: p }): ICSSInJSStyle => ({
    display: 'flex',
    justifyContent: p.mine ? 'flex-end' :  p.gutter ? 'flex-start' : 'center',
    position: 'relative',
    marginTop: _14pxAsRem_,
    marginBottom: _14pxAsRem_,
  }),

  gutter: ({ props: p }): ICSSInJSStyle => ({
    flex: 'none',
    display: p.mine ? 'none' : undefined,
    marginTop: _10pxAsRem_,
    marginBottom: _10pxAsRem_,
    marginLeft: p.mine ? _10pxAsRem_ : 0,
    marginRight: p.mine ? 0 : _10pxAsRem_,
  }),
  content: ({props: p}): ICSSInJSStyle => ({
    ...(!p.gutter && !p.mine && {
      flex: 1,
    }),
  }),
}

src/components/Chat/ChatItem.tsx

src/components/Chat/ChatGutter.tsx

docs/src/prototypes/chatPane/services/messageFactoryMock.tsx

docs/src/prototypes/chatPane/chatPaneContent.tsx

docs/src/examples/components/Chat/Types/ChatExample.tsx

mnajdova

Please take a look on the comments. Let's resolve/comment on them and push this forward.

bmdalex · 2018-12-12T16:37:52Z

Update on final changes based on latest sync with the team:

removed Chat.Gutter subcomponent because:
- it doesn’t bring any value, it's just another component to support
- it's not breaking any consistency or patterns as we have slots that do not have subcomponents (Button has icon slot but there is no Button.Icon)
- we use Slot instead
renamed Chat.Item content prop to message
deprecated children API examples

New API:

<Chat.Item
  gutterPosition='start'
  gutter={{ content: <Avatar …/> }}
  message={{ content: <Chat.Message …> }}
/>

- addressed comments - imporoved styles

… using children css selectors

- improved examples; - added fix for gutter being created by default;

- replaced Chat.Item content prop with message; - deprecated Children API;

src/components/Chat/ChatItem.tsx

mnajdova

Looks good, please just update the propTypes for the ChatMessage component (see #556 (comment)) You can merge after that.

bmdalex requested review from dvdzkwsk, kuzhelov, levithomason, miroslavstastny and mnajdova as code owners December 4, 2018 18:34

bmdalex added the 🚀 ready for review label Dec 4, 2018

bmdalex force-pushed the feat/chat-gutter branch from 28dffda to 67a4072 Compare December 5, 2018 10:38