[MM-204] Fixed issues/enhancements in link tooltip component (#457)

* [MM-204] Fixed issues/enhancements in link tooltip component 1. Removed the "see more" link. 2. Fixed branch name overflow. 3. Added limit to title and description. 4. Displayed proper icon for a closed PR. 5. Updated the icons to match with the icons on gitlab.com. * [MM-204] Review fixes * [MM-204] Review fixes: 1. Renamed constants. 2. Created a util function for getting truncated text for title and description of tooltip. * [MM-204] Review fixes * [MM-228] Updated code to fix the length of tooltip description to three lines * [MM-229] Updated to check to match the URL hostname instead of complete URL with protocol * [MM-238] Review fixes * [MM-238] Review fixes: 1. Converted tooltip component file to typescript 2. Some code refactoring * [MM-238] Converted tooltip testcases file to typescript * [MM-262] Fix icon allignment in tooltip
mattermost · Mar 8, 2024 · 789a404 · 789a404
1 parent 387d84d
commit 789a404
Show file tree

Hide file tree

Showing 15 changed files with 298 additions and 194 deletions.
diff --git a/webapp/src/components/link_tooltip/index.js b/webapp/src/components/link_tooltip/index.js
diff --git a/.../components/link_tooltip/link_tooltip.jsx → webapp/src/components/link_tooltip/index.tsx b/.../components/link_tooltip/link_tooltip.jsx → webapp/src/components/link_tooltip/index.tsx
@@ -1,49 +1,50 @@
 import React, {useEffect, useState} from 'react';
-import PropTypes from 'prop-types';
 import ReactMarkdown from 'react-markdown';
-import {GitMergeIcon, GitPullRequestIcon, IssueClosedIcon, IssueOpenedIcon} from '@primer/octicons-react';
-
-import {useDispatch} from 'react-redux';
+import {useDispatch, useSelector} from 'react-redux';
 
 import {logError} from 'mattermost-redux/actions/errors';
 
+import {GitLabIssueOpenIcon, GitLabMergeRequestIcon, GitLabMergeRequestClosedIcon, GitLabMergedIcon, GitLabIssueClosedIcon} from '../../utils/icons';
+
 import Client from '../../client';
-import {validateGitlabURL} from '../../utils/regex_utils';
-import {isValidUrl} from '../../utils/url_utils';
+import {getTruncatedText, validateGitlabUrl, isValidUrl, getInfoAboutLink} from '../../utils/tooltip_utils';
+import {TooltipData} from 'src/types/gitlab_items';
+import {getConnected, getConnectedGitlabUrl} from 'src/selectors';
 
 import './tooltip.css';
 
 const STATE_COLOR_MAP = {
     OPENED_COLOR: '#28a745',
     CLOSED_COLOR: '#cb2431',
     MERGED_COLOR: '#6f42c1',
+    ISSUE_CLOSED_COLOR: '#0b5cad',
 };
 
 const STATE_TYPES = {
     OPENED: 'opened',
     CLOSED: 'closed',
+    MERGED: 'merged',
 };
 
 const LINK_TYPES = {
     MERGE_REQUESTS: 'merge_requests',
     ISSUES: 'issues',
 };
 
-export const getInfoAboutLink = (href, hostname) => {
-    const linkInfo = href.split(`${hostname}/`)[1].split('/');
-    if (linkInfo.length >= 5) {
-        return {
-            owner: linkInfo[0],
-            repo: linkInfo[1],
-            type: linkInfo[3],
-            number: linkInfo[4],
-        };
-    }
-    return {};
-};
+const TOOLTIP_ICON_DIMENSION = 16;
+const TOOLTIP_MAX_TITLE_LENGTH = 70;
+
+type Props = {
+    href: string;
+    show: boolean;
+}
+
+const LinkTooltip = ({href, show}: Props) => {
+    const [data, setData] = useState<TooltipData | null>(null);
+
+    const connected = useSelector(getConnected);
+    const connectedGitlabUrl = useSelector(getConnectedGitlabUrl);
 
-export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
-    const [data, setData] = useState(null);
     const dispatch = useDispatch();
     useEffect(() => {
         if (!connected || !show) {
@@ -55,8 +56,9 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
         }
 
         const url = new URL(href);
+        const gitlabUrl = new URL(connectedGitlabUrl);
         const init = async () => {
-            if (url.origin === gitlabURL && validateGitlabURL(href)) {
+            if (url.host === gitlabUrl.host && validateGitlabUrl(href)) {
                 const linkInfo = getInfoAboutLink(href, url.hostname);
                 let res;
                 switch (linkInfo?.type) {
@@ -67,7 +69,7 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
                     res = await Client.getPullRequest(linkInfo.owner, linkInfo.repo, linkInfo.number);
                     break;
                 default:
-                    dispatch(logError(`link type ${linkInfo?.type} is not supported to display a tooltip`));
+                    dispatch(logError({message: `link type ${linkInfo.type} is not supported to display a tooltip`}));
                     return;
                 }
 
@@ -81,34 +83,35 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
         init();
     }, [connected, href, show]);
 
-    const getIconElement = () => {
-        const iconProps = {
-            size: 'small',
-            verticalAlign: 'text-bottom',
-        };
+    if (!data || !show) {
+        return null;
+    }
 
+    const getIconElement = () => {
         let color;
         let icon;
-        const {OPENED_COLOR, CLOSED_COLOR, MERGED_COLOR} = STATE_COLOR_MAP;
+        const {OPENED_COLOR, CLOSED_COLOR, MERGED_COLOR, ISSUE_CLOSED_COLOR} = STATE_COLOR_MAP;
         switch (data.type) {
         case LINK_TYPES.MERGE_REQUESTS:
-            color = OPENED_COLOR;
-            icon = <GitPullRequestIcon {...iconProps}/>;
+            icon = (
+                <GitLabMergeRequestIcon
+                    fill={OPENED_COLOR}
+                    width={TOOLTIP_ICON_DIMENSION}
+                    height={TOOLTIP_ICON_DIMENSION}
+                />
+            );
             if (data.state === STATE_TYPES.CLOSED) {
-                if (data.merged) {
-                    color = MERGED_COLOR;
-                    icon = <GitMergeIcon {...iconProps}/>;
-                } else {
-                    color = CLOSED_COLOR;
-                }
+                icon = <GitLabMergeRequestClosedIcon fill={CLOSED_COLOR}/>;
+            } else if (data.state === STATE_TYPES.MERGED) {
+                icon = <GitLabMergedIcon fill={MERGED_COLOR}/>;
             }
             break;
         case LINK_TYPES.ISSUES:
             color = data.state === STATE_TYPES.OPENED ? OPENED_COLOR : CLOSED_COLOR;
-            icon = data.state === STATE_TYPES.OPENED ? <IssueOpenedIcon {...iconProps}/> : <IssueClosedIcon {...iconProps}/>;
+            icon = data.state === STATE_TYPES.OPENED ? <GitLabIssueOpenIcon fill={OPENED_COLOR}/> : <GitLabIssueClosedIcon fill={ISSUE_CLOSED_COLOR}/>;
             break;
         default:
-            dispatch(logError(`link type ${data.type} is not supported to display a tooltip`));
+            dispatch(logError({message: `link type ${data.type} is not supported to display a tooltip`}));
         }
 
         return (
@@ -118,10 +121,6 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
         );
     };
 
-    if (!data || !show) {
-        return null;
-    }
-
     const date = new Date(data.created_at).toDateString();
 
     return (
@@ -138,14 +137,14 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
                     <span>{date}</span>
                 </div>
 
-                <div className='body d-flex mt-2'>
-                    <span className='pt-1 pb-1 pr-2'>
+                <div className='body d-flex mt-1'>
+                    <span className='tooltip-icon'>
                         {getIconElement()}
                     </span>
 
                     <div className='tooltip-info mt-1'>
                         <a href={href}>
-                            <h5 className='mr-1'>{data.title}</h5>
+                            <h5 className='mr-1'>{getTruncatedText(data.title, TOOLTIP_MAX_TITLE_LENGTH)}</h5>
                             <span className='mr-number'>{`#${data.iid}`}</span>
                         </a>
                         <div className='markdown-text mt-1 mb-1'>
@@ -174,16 +173,6 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
                             </div>
                         )}
 
-                        <div className='see-more mt-1'>
-                            <a
-                                href={href}
-                                target='_blank'
-                                rel='noopener noreferrer'
-                            >
-                                {'See more'}
-                            </a>
-                        </div>
-
                         {/* Labels */}
                         <div className='labels mt-3'>
                             {data.labels && data.labels_with_details?.length && data.labels_with_details.map((label) => {
@@ -192,9 +181,9 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
                                         key={label.name}
                                         className='label mr-1'
                                         title={label.description}
-                                        style={{backgroundColor: label.color}}
+                                        style={{backgroundColor: label.color as string}}
                                     >
-                                        <span style={{color: label.text_color}}>{label.name}</span>
+                                        <span style={{color: label.text_color as string}}>{label.name}</span>
                                     </span>
                                 );
                             })}
@@ -206,9 +195,4 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
     );
 };
 
-LinkTooltip.propTypes = {
-    href: PropTypes.string.isRequired,
-    connected: PropTypes.bool.isRequired,
-    gitlabURL: PropTypes.string.isRequired,
-    show: PropTypes.bool,
-};
+export default LinkTooltip;
diff --git a/...onents/link_tooltip/link_tooltip.test.jsx → ...onents/link_tooltip/link_tooltip.test.tsx b/...onents/link_tooltip/link_tooltip.test.jsx → ...onents/link_tooltip/link_tooltip.test.tsx
@@ -1,4 +1,6 @@
-import {getInfoAboutLink} from './link_tooltip';
+import {describe, expect, it} from '@jest/globals';
+
+const {getInfoAboutLink} = require('../../utils/tooltip_utils');
 
 describe('getInfoAboutLink should work as expected', () => {
     it('Should return an object of correct owner, repo, type, and number when given a valid GitLab hostname and href with all required information', () => {

diff --git a/webapp/src/components/link_tooltip/tooltip.css b/webapp/src/components/link_tooltip/tooltip.css
@@ -15,9 +15,11 @@
 /* Gitlab Tooltip */
 .gitlab-tooltip .box {
     background-color: white;
-    box-shadow: 0 1px 15px rgba(27,31,35,.15);
     position: relative;
     width: 360px;
+    border: 1px solid rgba(var(--center-channel-color-rgb), 0.16);
+    box-shadow: 0 8px 24px rgba(0,0,0,.12);
+    border-radius: 4px;
 }
 
 .gitlab-tooltip--bottom-left, .gitlab-tooltip--top-left {
@@ -42,11 +44,23 @@
     line-height: 1.25;
 }
 
+/* Icon */
+.gitlab-tooltip .tooltip-icon {
+    margin: 5px 5px 0 0;
+}
+
 /* Info */
 .gitlab-tooltip .tooltip-info {
-    line-height: 1.25;
     display: flex;
     flex-direction: column;
+    font-family: Metropolis,sans-serif;
+    font-size: 16px;
+    line-height: 22px;
+}
+
+/* Info */
+.gitlab-tooltip .tooltip-info a {
+    line-height: normal;
 }
 
 .gitlab-tooltip .tooltip-info > a, .gitlab-tooltip .tooltip-info > a:hover {
@@ -70,10 +84,13 @@
     -ms-hyphens: auto;
     hyphens: auto;
     max-height: 150px;
-    line-height: 1.25;
     overflow: hidden;
     font-size: 12px;
     word-break: break-word;
+    line-height: 16px;
+    display: -webkit-box;
+    -webkit-line-clamp: 3;
+    -webkit-box-orient: vertical;  
 }
 
 .gitlab-tooltip .tooltip-info .markdown-text::-webkit-scrollbar {
@@ -97,7 +114,7 @@
     font-size: 12px;
     font-weight: 600;
     line-height: 15px;
-    border-radius: 2px;
+    border-radius: 4px;
     box-shadow: inset 0 -1px 0 rgba(27,31,35,.12);
     display: inline-block;
     overflow: hidden;
@@ -119,6 +136,8 @@
     color: var(--blue);
     white-space: nowrap;
     background-color: var(--light-blue);
-    border-radius: 3px;
+    border-radius: 4px;
     max-width: 140px;
+    text-overflow: ellipsis;
+    overflow: hidden;
 }
diff --git a/webapp/src/components/sidebar_buttons/button_icons.tsx b/webapp/src/components/sidebar_buttons/button_icons.tsx
diff --git a/webapp/src/components/sidebar_buttons/sidebar_buttons.tsx b/webapp/src/components/sidebar_buttons/sidebar_buttons.tsx
@@ -10,7 +10,7 @@ import {Theme} from 'mattermost-redux/types/preferences';
 import {RHSStates, connectUsingBrowserMessage} from '../../constants';
 import {isDesktopApp} from '../../utils/user_agent';
 
-import {GitLabIssuesIcon, GitLabMergeRequestIcon, GitLabReviewsIcon, GitLabTodosIcon} from './button_icons';
+import {GitLabIssuesIcon, GitLabMergeRequestIcon, GitLabReviewsIcon, GitLabTodosIcon} from '../../utils/icons';
 
 interface SidebarButtonsProps {
     theme: Theme;

diff --git a/webapp/src/selectors/index.js b/webapp/src/selectors/index.js
@@ -58,3 +58,7 @@ export const getSidebarData = createSelector(
         };
     },
 );
+
+export const getConnected = (state) => state[`plugins-${manifest.id}`].connected;
+
+export const getConnectedGitlabUrl = (state) => state[`plugins-${manifest.id}`].gitlabURL;