mattermost-plugin-gitlab icon indicating copy to clipboard operation
mattermost-plugin-gitlab copied to clipboard

[MM-204] Fixed issues/enhancements in link tooltip component

Open raghavaggarwal2308 opened this issue 1 year ago • 15 comments

Summary

  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.

Screenshot:

image

What to test:

  1. The "see more" link is not visible in the tooltip.
  2. The branch, label, title, and description are not overflowing.
  3. Proper icons are displayed for each type of issue/PR.
  4. Displayed icons are matching with the icons on Gitlab.
  5. Updated check to match the URL hostname instead of complete URL with protocol.
Steps to test:
  1. Connect your GitLab account.
  2. Hover over a GitLab PR/issue link in a channel.

Ticket Link

Fixes: https://community.mattermost.com/core/pl/rybgwy37xibpjgsqjrd8yxkkbo

raghavaggarwal2308 avatar Feb 09 '24 10:02 raghavaggarwal2308

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 33.43%. Comparing base (175c61d) to head (c5f231b). Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #457      +/-   ##
==========================================
+ Coverage   33.38%   33.43%   +0.04%     
==========================================
  Files          22       22              
  Lines        4014     4008       -6     
==========================================
  Hits         1340     1340              
+ Misses       2541     2535       -6     
  Partials      133      133              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 09 '24 10:02 codecov[bot]

@matthewbirtch Updated

raghavaggarwal2308 avatar Feb 13 '24 09:02 raghavaggarwal2308

@avas27JTG Fixed the review comments mentioned by you

raghavaggarwal2308 avatar Feb 13 '24 11:02 raghavaggarwal2308

@matthewbirtch Updated

Thanks @raghavaggarwal2308 when you have a moment could provide an updated screenshot?

matthewbirtch avatar Feb 14 '24 17:02 matthewbirtch

@matthewbirtch Updated

Thanks @raghavaggarwal2308 when you have a moment could provide an updated screenshot?

@matthewbirtch I have updated the screenshot in the PR description now 👍🏽

raghavaggarwal2308 avatar Feb 14 '24 17:02 raghavaggarwal2308

@matthewbirtch I have updated the screenshot in the PR description now 👍🏽

Looks good @raghavaggarwal2308, thanks. Is there any way we can truncate the description to be max 3 lines rather than having a few words overflow on to the 4th line?

matthewbirtch avatar Feb 14 '24 21:02 matthewbirtch

@matthewbirtch I have updated the screenshot in the PR description now 👍🏽

Looks good @raghavaggarwal2308, thanks. Is there any way we can truncate the description to be max 3 lines rather than having a few words overflow on to the 4th line?

@matthewbirtch Yes, it is possible but it will be a little complex to do also it will make the component rendering a little slow. A hack for this would be to fix the height of the description area and hide the remaining content but in that case, we will not be able to add the ... at the end.

raghavaggarwal2308 avatar Feb 15 '24 09:02 raghavaggarwal2308

@matthewbirtch I have updated the screenshot in the PR description now 👍🏽

Looks good @raghavaggarwal2308, thanks. Is there any way we can truncate the description to be max 3 lines rather than having a few words overflow on to the 4th line?

@matthewbirtch Yes, it is possible but it will be a little complex to do also it will make the component rendering a little slow. A hack for this would be to fix the height of the description area and hide the remaining content but in that case, we will not be able to add the ... at the end.

Ok, I'm not sure if this is cross-browser compatible, but there is a line-clamp property that works for webkit. -webkit-line-clamp: 3;

matthewbirtch avatar Feb 15 '24 17:02 matthewbirtch

@matthewbirtch These are the versions for browser compatibility of this property. If these look good to you we can make the change image

raghavaggarwal2308 avatar Feb 16 '24 13:02 raghavaggarwal2308

@matthewbirtch These are the versions for browser compatibility of this property. If these look good to you we can make the change

I think we should be good, since these are minimum requirements we currently publish: image

I believe we are using this CSS property in the webapp already in a few other locations anyway.

matthewbirtch avatar Feb 16 '24 14:02 matthewbirtch

@matthewbirtch Updated the CSS

raghavaggarwal2308 avatar Feb 19 '24 09:02 raghavaggarwal2308

@matthewbirtch Updated the CSS

Thanks @raghavaggarwal2308. Could you share an updated screenshot again?

matthewbirtch avatar Feb 19 '24 16:02 matthewbirtch

@matthewbirtch Here is the updated screenshot image

raghavaggarwal2308 avatar Feb 20 '24 07:02 raghavaggarwal2308

Thanks @raghavaggarwal2308 looks good to me

matthewbirtch avatar Feb 20 '24 14:02 matthewbirtch

@mickmister Fixed the review comments added by you

raghavaggarwal2308 avatar Feb 21 '24 12:02 raghavaggarwal2308

@AayushChaudhary0001 The issue is fixed now. Please re-review.

raghavaggarwal2308 avatar Mar 05 '24 13:03 raghavaggarwal2308

Tested and working fine now for the positioning of icons also, LGTM. Approved

AayushChaudhary0001 avatar Mar 06 '24 11:03 AayushChaudhary0001