Mattermost-server: [Help Wanted] [MM-13298] Web: Remove usage of localizeMessage for aria-label and title attribues

[esethna]

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

---

Notes: Jira ticket

If possible, this should be replaced by using a FormattedMessage, but if that won't work, we should change these to get the intl object either from this.context or from injectIntl and then use that to perform the translation.

At the time this ticket was created, this happens in many areas, notably:

• SVG icons in components/svg
• icons in settings menus
• icons in the channel header
• icons in the channel sidebar

[kosgrz]

@esethna I can take this one.

[hanzei]

Thanks @kosgrz! :tada:

[kosgrz]

@hanzei
When I was testing my changes, I found an issue on the right sidebar. When you hover the mouse over the expand icon, two tooltips appear:

The upper tooltip is created by the OverlayTrigger component. The lower is just a text from the title attribute of the icon element. I don't think it's correct behavior. The OverlayTrigger or the title attribute from the icon should be deleted. Which tooltip should be shown on mouse hover? I could fix that.

[hanzei]

Nice catch @kosgrz!
I think only OverlayTrigger should be shown. @saturninoabril Can you confirm this?

[saturninoabril]

@hanzei is right. Use overlay attribute of OverlayTrigger for tooltip and delete title attribute causing duplicate tooltip.

Thanks @kosgrz!

[kosgrz]

@saturninoabril thanks for the answer!

I have one more question. For instance, there is a PermalinkView component which uses localizeMessage method. There is the code fragment of the usage:

<i
    className='fa fa-arrow-down'
    title={localizeMessage('center_panel.recent.icon', 'Jump to recent messages Icon')}
/>

If I want to remove the localizeMessage method, I can use the FormattedMessage instead. The code would look like this:

<FormattedMessage
    id='center_panel.recent.icon'
    defaultMessage='Jump to recent messages Icon'
>
    {(title) => (
        <i
            className='fa fa-arrow-down'
            title={title}
        />
    )}
</FormattedMessage>

This will be working perfectly fine. Only issue will be with tests using shallow rendering. Snapshoots will look like this:

<FormattedMessage
    defaultMessage="Jump to recent messages Icon"
    id="center_panel.recent.icon"
    values={Object {}}
>
    <Component />
</FormattedMessage>

There will be no simple way to check whether the FormattedMessage contains a i tag with proper title and className attributes.

Alternatively, I can wrap the component by injectIntl and use intl.formatMessage instead of localizeMessage.

Which way is better in your opinion? Should I use FormattedMessage or intl.formatMessage?

[saturninoabril]

In that case, I'm incline to use formatMessage, but instead of wrapping with injectIntl, pass intlShape as context (like here)

For unit testing,

jest.mock('react-intl'); // place this mock up on top
...

const wrapper = shallow(
    <Component {...baseProps}/>,
    {context: {intl: {formatMessage: jest.fn()}}},
);

[kosgrz]

@saturninoabril should I use formatMessage only in files with shallow tests or everywhere? I think it's better to use formatMessage everywhere because it would be easier to write new tests in the future. What do you think?

[saturninoabril]

I agree with you but I'd say use formatMessage as necessary only like in above case. The reason being is that FormattedMessage component is straight forward and easy to implement/read than formatMessage API under context.

[saturninoabril]

Thanks @kosgrz! PR has been merged.