feat(Chat): add actionMenu prop for ChatMessage

[layershifter]

actions vs actionMenu

Initially, I was thinking about actions, but this shorthand points to Menu component and it was really confusing why the code below will be broken:

<ChatMessage actions={['one', 'two']} />

---

Also picks changes from #765.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@6352b50). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #811   +/-   ##
=========================================
  Coverage          ?   93.54%           
=========================================
  Files             ?       21           
  Lines             ?      728           
  Branches          ?       69           
=========================================
  Hits              ?      681           
  Misses            ?       47           
  Partials          ?        0

---

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 6352b50...5e3befb. Read the comment docs.

[kuzhelov]

just putting my vote on that: would rather prefer actions than actionsMenu for the following reasons

• it will be easier to avoid breaking changes in future - for example, if at some point it will become an actionsPanel
• it is consistent with the naming taken for Attachment component, where there is an action slot

[kuzhelov]

but given the example from description, agree that it is definitely better to use singular menu noun in the name of the prop, to properly reflect underlying semantics

[kuzhelov]

actually, a bit surprised that we are not handling array as 'primitive' shorthand values - is there any particular reason for that? Might expect that this feature is necessary for all the places where collection-container component stays as a default one for shorthand value

[layershifter]

I can be wrong, but this decision came from the consistency point: you can pass only ShorthandValue or ShorthandValue[]. However, I understand your point, had the same feeling about it.

[kuzhelov]

got it, thank you @layershifter! actually, this original problem (actions and necessity to compromise on actionsMenu) is a good example that might serve as an inspiration for us to further refine this moment in future

[kuzhelov]

could you, please, suggest the case we would like to cover with that?

[layershifter]

It comes from the accessibility point: when we will click Enter key, the focus will be moved to another element.

With

Without

[kuzhelov]

then, frankly, I am a bit confused with onBlur event being raised unconditionally - so, for instance, even if shouldPreserveFocusState is true (and, as a consequence, even this.state.focused remains to be true) - even in this situation we will trigger blur event for ChatMessage.

So, couple of things that I would like to solve here:

• ensure that I am not missing anything in this thought
• and, in case if so, it seems that we need to resolve this emerged problem that is about implicit reliance of events subscriptions - we should strive for component's behavioural intents being explicit

Could you, please, to resolve this issue for now, just provide a comment that will describe the intent of these lines? Thank you!

[layershifter]

this.state.focused controls is focused the ChatMessage on any of its children, when we're navigating with keyboard focused elements changes and there is no way to use :focus:

May be proper naming can make it better?

[kuzhelov]

yes, totally agree about naming - this should help. However, it seems that many things are involved here, so it is, probably, much better to address naming and all the related moments in a dedicated PR - what do you think?

[kuzhelov]

we need to update this one :)