Skip to content

Commit 789a404

Browse files
author
Raghav Aggarwal
authored
[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
1 parent 387d84d commit 789a404

File tree

15 files changed

+298
-194
lines changed

15 files changed

+298
-194
lines changed

‎webapp/src/components/link_tooltip/index.js‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

webapp/src/components/link_tooltip/link_tooltip.jsx renamed to webapp/src/components/link_tooltip/index.tsx

Lines changed: 47 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,50 @@
11
import React, {useEffect, useState} from 'react';
2-
import PropTypes from 'prop-types';
32
import ReactMarkdown from 'react-markdown';
4-
import {GitMergeIcon, GitPullRequestIcon, IssueClosedIcon, IssueOpenedIcon} from '@primer/octicons-react';
5-
6-
import {useDispatch} from 'react-redux';
3+
import {useDispatch, useSelector} from 'react-redux';
74

85
import {logError} from 'mattermost-redux/actions/errors';
96

7+
import {GitLabIssueOpenIcon, GitLabMergeRequestIcon, GitLabMergeRequestClosedIcon, GitLabMergedIcon, GitLabIssueClosedIcon} from '../../utils/icons';
8+
109
import Client from '../../client';
11-
import {validateGitlabURL} from '../../utils/regex_utils';
12-
import {isValidUrl} from '../../utils/url_utils';
10+
import {getTruncatedText, validateGitlabUrl, isValidUrl, getInfoAboutLink} from '../../utils/tooltip_utils';
11+
import {TooltipData} from 'src/types/gitlab_items';
12+
import {getConnected, getConnectedGitlabUrl} from 'src/selectors';
1313

1414
import './tooltip.css';
1515

1616
const STATE_COLOR_MAP = {
1717
OPENED_COLOR: '#28a745',
1818
CLOSED_COLOR: '#cb2431',
1919
MERGED_COLOR: '#6f42c1',
20+
ISSUE_CLOSED_COLOR: '#0b5cad',
2021
};
2122

2223
const STATE_TYPES = {
2324
OPENED: 'opened',
2425
CLOSED: 'closed',
26+
MERGED: 'merged',
2527
};
2628

2729
const LINK_TYPES = {
2830
MERGE_REQUESTS: 'merge_requests',
2931
ISSUES: 'issues',
3032
};
3133

32-
export const getInfoAboutLink = (href, hostname) => {
33-
const linkInfo = href.split(`${hostname}/`)[1].split('/');
34-
if (linkInfo.length >= 5) {
35-
return {
36-
owner: linkInfo[0],
37-
repo: linkInfo[1],
38-
type: linkInfo[3],
39-
number: linkInfo[4],
40-
};
41-
}
42-
return {};
43-
};
34+
const TOOLTIP_ICON_DIMENSION = 16;
35+
const TOOLTIP_MAX_TITLE_LENGTH = 70;
36+
37+
type Props = {
38+
href: string;
39+
show: boolean;
40+
}
41+
42+
const LinkTooltip = ({href, show}: Props) => {
43+
const [data, setData] = useState<TooltipData | null>(null);
44+
45+
const connected = useSelector(getConnected);
46+
const connectedGitlabUrl = useSelector(getConnectedGitlabUrl);
4447

45-
export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
46-
const [data, setData] = useState(null);
4748
const dispatch = useDispatch();
4849
useEffect(() => {
4950
if (!connected || !show) {
@@ -55,8 +56,9 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
5556
}
5657

5758
const url = new URL(href);
59+
const gitlabUrl = new URL(connectedGitlabUrl);
5860
const init = async () => {
59-
if (url.origin === gitlabURL && validateGitlabURL(href)) {
61+
if (url.host === gitlabUrl.host && validateGitlabUrl(href)) {
6062
const linkInfo = getInfoAboutLink(href, url.hostname);
6163
let res;
6264
switch (linkInfo?.type) {
@@ -67,7 +69,7 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
6769
res = await Client.getPullRequest(linkInfo.owner, linkInfo.repo, linkInfo.number);
6870
break;
6971
default:
70-
dispatch(logError(`link type ${linkInfo?.type} is not supported to display a tooltip`));
72+
dispatch(logError({message: `link type ${linkInfo.type} is not supported to display a tooltip`}));
7173
return;
7274
}
7375

@@ -81,34 +83,35 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
8183
init();
8284
}, [connected, href, show]);
8385

84-
const getIconElement = () => {
85-
const iconProps = {
86-
size: 'small',
87-
verticalAlign: 'text-bottom',
88-
};
86+
if (!data || !show) {
87+
return null;
88+
}
8989

90+
const getIconElement = () => {
9091
let color;
9192
let icon;
92-
const {OPENED_COLOR, CLOSED_COLOR, MERGED_COLOR} = STATE_COLOR_MAP;
93+
const {OPENED_COLOR, CLOSED_COLOR, MERGED_COLOR, ISSUE_CLOSED_COLOR} = STATE_COLOR_MAP;
9394
switch (data.type) {
9495
case LINK_TYPES.MERGE_REQUESTS:
95-
color = OPENED_COLOR;
96-
icon = <GitPullRequestIcon {...iconProps}/>;
96+
icon = (
97+
<GitLabMergeRequestIcon
98+
fill={OPENED_COLOR}
99+
width={TOOLTIP_ICON_DIMENSION}
100+
height={TOOLTIP_ICON_DIMENSION}
101+
/>
102+
);
97103
if (data.state === STATE_TYPES.CLOSED) {
98-
if (data.merged) {
99-
color = MERGED_COLOR;
100-
icon = <GitMergeIcon {...iconProps}/>;
101-
} else {
102-
color = CLOSED_COLOR;
103-
}
104+
icon = <GitLabMergeRequestClosedIcon fill={CLOSED_COLOR}/>;
105+
} else if (data.state === STATE_TYPES.MERGED) {
106+
icon = <GitLabMergedIcon fill={MERGED_COLOR}/>;
104107
}
105108
break;
106109
case LINK_TYPES.ISSUES:
107110
color = data.state === STATE_TYPES.OPENED ? OPENED_COLOR : CLOSED_COLOR;
108-
icon = data.state === STATE_TYPES.OPENED ? <IssueOpenedIcon {...iconProps}/> : <IssueClosedIcon {...iconProps}/>;
111+
icon = data.state === STATE_TYPES.OPENED ? <GitLabIssueOpenIcon fill={OPENED_COLOR}/> : <GitLabIssueClosedIcon fill={ISSUE_CLOSED_COLOR}/>;
109112
break;
110113
default:
111-
dispatch(logError(`link type ${data.type} is not supported to display a tooltip`));
114+
dispatch(logError({message: `link type ${data.type} is not supported to display a tooltip`}));
112115
}
113116

114117
return (
@@ -118,10 +121,6 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
118121
);
119122
};
120123

121-
if (!data || !show) {
122-
return null;
123-
}
124-
125124
const date = new Date(data.created_at).toDateString();
126125

127126
return (
@@ -138,14 +137,14 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
138137
<span>{date}</span>
139138
</div>
140139

141-
<div className='body d-flex mt-2'>
142-
<span className='pt-1 pb-1 pr-2'>
140+
<div className='body d-flex mt-1'>
141+
<span className='tooltip-icon'>
143142
{getIconElement()}
144143
</span>
145144

146145
<div className='tooltip-info mt-1'>
147146
<a href={href}>
148-
<h5 className='mr-1'>{data.title}</h5>
147+
<h5 className='mr-1'>{getTruncatedText(data.title, TOOLTIP_MAX_TITLE_LENGTH)}</h5>
149148
<span className='mr-number'>{`#${data.iid}`}</span>
150149
</a>
151150
<div className='markdown-text mt-1 mb-1'>
@@ -174,16 +173,6 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
174173
</div>
175174
)}
176175

177-
<div className='see-more mt-1'>
178-
<a
179-
href={href}
180-
target='_blank'
181-
rel='noopener noreferrer'
182-
>
183-
{'See more'}
184-
</a>
185-
</div>
186-
187176
{/* Labels */}
188177
<div className='labels mt-3'>
189178
{data.labels && data.labels_with_details?.length && data.labels_with_details.map((label) => {
@@ -192,9 +181,9 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
192181
key={label.name}
193182
className='label mr-1'
194183
title={label.description}
195-
style={{backgroundColor: label.color}}
184+
style={{backgroundColor: label.color as string}}
196185
>
197-
<span style={{color: label.text_color}}>{label.name}</span>
186+
<span style={{color: label.text_color as string}}>{label.name}</span>
198187
</span>
199188
);
200189
})}
@@ -206,9 +195,4 @@ export const LinkTooltip = ({href, connected, gitlabURL, show}) => {
206195
);
207196
};
208197

209-
LinkTooltip.propTypes = {
210-
href: PropTypes.string.isRequired,
211-
connected: PropTypes.bool.isRequired,
212-
gitlabURL: PropTypes.string.isRequired,
213-
show: PropTypes.bool,
214-
};
198+
export default LinkTooltip;

webapp/src/components/link_tooltip/link_tooltip.test.jsx renamed to webapp/src/components/link_tooltip/link_tooltip.test.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import {getInfoAboutLink} from './link_tooltip';
1+
import {describe, expect, it} from '@jest/globals';
2+
3+
const {getInfoAboutLink} = require('../../utils/tooltip_utils');
24

35
describe('getInfoAboutLink should work as expected', () => {
46
it('Should return an object of correct owner, repo, type, and number when given a valid GitLab hostname and href with all required information', () => {

‎webapp/src/components/link_tooltip/tooltip.css‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
/* Gitlab Tooltip */
1616
.gitlab-tooltip .box {
1717
background-color: white;
18-
box-shadow: 0 1px 15px rgba(27,31,35,.15);
1918
position: relative;
2019
width: 360px;
20+
border: 1px solid rgba(var(--center-channel-color-rgb), 0.16);
21+
box-shadow: 0 8px 24px rgba(0,0,0,.12);
22+
border-radius: 4px;
2123
}
2224

2325
.gitlab-tooltip--bottom-left, .gitlab-tooltip--top-left {
@@ -42,11 +44,23 @@
4244
line-height: 1.25;
4345
}
4446

47+
/* Icon */
48+
.gitlab-tooltip .tooltip-icon {
49+
margin: 5px 5px 0 0;
50+
}
51+
4552
/* Info */
4653
.gitlab-tooltip .tooltip-info {
47-
line-height: 1.25;
4854
display: flex;
4955
flex-direction: column;
56+
font-family: Metropolis,sans-serif;
57+
font-size: 16px;
58+
line-height: 22px;
59+
}
60+
61+
/* Info */
62+
.gitlab-tooltip .tooltip-info a {
63+
line-height: normal;
5064
}
5165

5266
.gitlab-tooltip .tooltip-info > a, .gitlab-tooltip .tooltip-info > a:hover {
@@ -70,10 +84,13 @@
7084
-ms-hyphens: auto;
7185
hyphens: auto;
7286
max-height: 150px;
73-
line-height: 1.25;
7487
overflow: hidden;
7588
font-size: 12px;
7689
word-break: break-word;
90+
line-height: 16px;
91+
display: -webkit-box;
92+
-webkit-line-clamp: 3;
93+
-webkit-box-orient: vertical;
7794
}
7895

7996
.gitlab-tooltip .tooltip-info .markdown-text::-webkit-scrollbar {
@@ -97,7 +114,7 @@
97114
font-size: 12px;
98115
font-weight: 600;
99116
line-height: 15px;
100-
border-radius: 2px;
117+
border-radius: 4px;
101118
box-shadow: inset 0 -1px 0 rgba(27,31,35,.12);
102119
display: inline-block;
103120
overflow: hidden;
@@ -119,6 +136,8 @@
119136
color: var(--blue);
120137
white-space: nowrap;
121138
background-color: var(--light-blue);
122-
border-radius: 3px;
139+
border-radius: 4px;
123140
max-width: 140px;
141+
text-overflow: ellipsis;
142+
overflow: hidden;
124143
}

‎webapp/src/components/sidebar_buttons/button_icons.tsx‎

Lines changed: 0 additions & 65 deletions
This file was deleted.

‎webapp/src/components/sidebar_buttons/sidebar_buttons.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {Theme} from 'mattermost-redux/types/preferences';
1010
import {RHSStates, connectUsingBrowserMessage} from '../../constants';
1111
import {isDesktopApp} from '../../utils/user_agent';
1212

13-
import {GitLabIssuesIcon, GitLabMergeRequestIcon, GitLabReviewsIcon, GitLabTodosIcon} from './button_icons';
13+
import {GitLabIssuesIcon, GitLabMergeRequestIcon, GitLabReviewsIcon, GitLabTodosIcon} from '../../utils/icons';
1414

1515
interface SidebarButtonsProps {
1616
theme: Theme;

‎webapp/src/selectors/index.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,7 @@ export const getSidebarData = createSelector(
5858
};
5959
},
6060
);
61+
62+
export const getConnected = (state) => state[`plugins-${manifest.id}`].connected;
63+
64+
export const getConnectedGitlabUrl = (state) => state[`plugins-${manifest.id}`].gitlabURL;

0 commit comments

Comments
 (0)