[MM-13298] Remove usage of localizeMessage for aria-label and title attributes

[kosgrz]

Summary

Remove usage of localizeMessage for aria-label and title attributes in HTML tags. Remove duplicate tooltip caused by the usage of OverlayTrigger and title attribute.

Ticket Link

Jira ticket | GitHub Issue #9948

Checklist

• Ran make check-style to check for style errors (required for all pull requests)
• Ran make test to ensure unit and component tests passed
• Added or updated unit tests (required for all new features)

[saturninoabril]

Thanks @kosgrz, looks good to me. Just one small comment re: one missed localizeMessage.

Also, looks like we can have a collection of icons, like below to name a few:

• Success Icon
• Reload Icon
• Collapse Icon
• Loading Icon
• Warning Icon

That might be good to have here, or a separate PR will do as well.

[kosgrz]

Thanks for the review @saturninoabril.

Also, looks like we can have a collection of icons, like below to name a few:
Success Icon
Reload Icon
Collapse Icon
Loading Icon
Warning Icon

I don't know I understand correctly. Do you mean I should extract these icons to separate components and use them in proper places?

So for example if I have this code fragment in cluster_settings.jsx:

if (this.state.showWarning) {
    warning = (
        <div
            style={style.warning}
            className='alert alert-warning'
        >
            <FormattedMessage
                id='generic_icons.warning'
                defaultMessage='Warning Icon'
            >
                {(title) => (
                    <i
                        className='fa fa-warning'
                        title={title}
                    />
                )}
            </FormattedMessage>
            <FormattedMarkdownMessage
                id='admin.cluster.should_not_change'
                defaultMessage='WARNING: These settings may...'
            />
        </div>
    );
}

I instead should extract FormattedMessage with the i tag to a new React component named WarningIcon with the following content:

export default class WarningIcon extends React.PureComponent {
    render() {
        return (
            <FormattedMessage
                id='generic_icons.warning'
                defaultMessage='Warning Icon'
            >
                {(title) => (
                    <i
                        className='fa fa-warning'
                        title={title}
                    />
                )}
            </FormattedMessage>
        );
    }
}

and use it in cluster_settings.jsx as follows:

if (this.state.showWarning) {
    warning = (
        <div
            style={style.warning}
            className='alert alert-warning'
        >
            <WarningIcon/>
            <FormattedMarkdownMessage
                id='admin.cluster.should_not_change'
                defaultMessage='WARNING: These settings may...'
            />
        </div>
    );
}

Is my understanding correct?

[saturninoabril]

@kosgrz Yes, that's exactly. Maybe create a folder like /components/icon or rename /components/svg to /components/icon and put all those new components into it.

UPDATE: I'll bring up first the case of moving other components from components/svg to Devs and will deal with it separately in case.

[hmhealey]

I agree that we can move the other icons later. It probably makes sense to consolidate them, but I think it's fine leaving them where they are for now

[saturninoabril]

Thanks @kosgrz, all are good to me except to last comments on function binding and snapshot tests for new icon components.

[saturninoabril]

In case this needs to bind, instead of adding above line you may convert the function into arrow function.

renderOption = (option, isSelected, onAdd) => {...}

[kosgrz]

Thanks for the notice. I forgot about this benefit of arrow functions when I was writing this code. Nevertheless, this line is now unnecessary since I use just the AddIcon component inside renderOption instead of this.context.intl.formatMessage and I don't need to have access to this. I deleted this line in the last update of this PR.

[saturninoabril]

No action for now.
In the future, this needs to update once our UI come up with the standard dropdown icon of either fa-chevron-down or fa-angle-down.

Ticket - https://mattermost.atlassian.net/browse/MM-13758

[saturninoabril]

Thanks @kosgrz for your contribution!