mattermost-plugin-gitlab
mattermost-plugin-gitlab copied to clipboard
[MM-204] Fixed issues/enhancements in link tooltip component
Summary
- Removed the "see more" link.
- Fixed branch name overflow.
- Added limit to title and description.
- Displayed proper icon for a closed PR.
- Updated the icons to match with the icons on gitlab.com.
Screenshot:
What to test:
- The "see more" link is not visible in the tooltip.
- The branch, label, title, and description are not overflowing.
- Proper icons are displayed for each type of issue/PR.
- Displayed icons are matching with the icons on Gitlab.
- Updated check to match the URL hostname instead of complete URL with protocol.
Steps to test:
- Connect your GitLab account.
- Hover over a GitLab PR/issue link in a channel.
Ticket Link
Fixes: https://community.mattermost.com/core/pl/rybgwy37xibpjgsqjrd8yxkkbo
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.
@matthewbirtch Updated
@avas27JTG Fixed the review comments mentioned by you
@matthewbirtch Updated
Thanks @raghavaggarwal2308 when you have a moment could provide an updated screenshot?
@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 👍🏽
@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 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.
@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 These are the versions for browser compatibility of this property. If these look good to you we can make the change
@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:
I believe we are using this CSS property in the webapp already in a few other locations anyway.
@matthewbirtch Updated the CSS
@matthewbirtch Updated the CSS
Thanks @raghavaggarwal2308. Could you share an updated screenshot again?
@matthewbirtch Here is the updated screenshot
Thanks @raghavaggarwal2308 looks good to me
@mickmister Fixed the review comments added by you
@AayushChaudhary0001 The issue is fixed now. Please re-review.
Tested and working fine now for the positioning of icons also, LGTM. Approved