Made GitHub username clickable #424

djanda97 · 2021-03-07T23:59:38Z

Summary

Added the userProfile variable to hold the user's GitHub profile URL.
Wrapped the userName variable in <a></a> with a link to the URL provided from the userProfile variable.

Ticket Link

Fixes #408

mattermod · 2021-03-07T23:59:42Z

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

djanda97 · 2021-03-08T00:02:02Z

@mickmister I haven't setup my environment for this plugin to test my changes locally yet, but is this the sort of addition that you had in mind for #408?

mickmister

Thanks @djanda97! I have just a few requests

mickmister · 2021-03-08T02:00:43Z

webapp/src/components/sidebar_right/github_items.jsx


        if (item.user) {
            userName = item.user.login;
+            userProfile = `https://github.com/${userName}`;


What do you think about this instead:

if (item.user) { userName = item.user.login; } else if (item.owner) { userName = item.owner.login; } const userProfile = `https://github.com/${userName}`;

Good catch!

Actually userName may not be defined. So maybe just construct the URL inline since it's only used once:

<a href={`https://github.com/${userName}`} > {userName} </a>

mickmister · 2021-03-08T02:08:46Z

webapp/src/components/sidebar_right/github_items.jsx

@@ -158,7 +160,7 @@ function GithubItems(props) {
                    style={style.subtitle}
                >
                    {item.created_at && ('Opened ' + formatTimeSince(item.created_at) + ' ago')}
-                    {userName && ' by ' + userName}
+                    {userName && <a href={userProfile}> by {userName}</a>}


The linter is complaining about the JSX/string formatting here:

163:56 error Missing JSX expression container around literal string: “by” react/jsx-no-literals

Maybe try something like this:

Suggested change

{userName && <a href={userProfile}> by {userName}</a>}

{userName && (

<>

{' by '}

<a href={userProfile}>{userName}</a>

</>

)}

Once I get my environment setup, I'll go ahead and test out this change.

mickmister · 2021-03-08T02:14:29Z

I tested out what the styling looks like when the username is wrapped in an anchor tag, and It looks like the username is a bit dark. I'm wondering if we can make it so that specific word is not dark, while the words around it are dark as they are now. @abhijit-singh Do you have any thoughts on this?

mickmister · 2021-03-08T02:18:58Z

@djanda97 Do you mind commenting on the ticket #408 so I may assign you?

Also feel free to ask questions on our community server at https://community.mattermost.com/core/channels/github-plugin e.g. questions on setting up the plugin.

abhijit-singh · 2021-03-16T08:50:32Z

I tested out what the styling looks like when the username is wrapped in an anchor tag, and It looks like the username is a bit dark. I'm wondering if we can make it so that specific word is not dark, while the words around it are dark as they are now. @abhijit-singh Do you have any thoughts on this?

@mickmister I agree that the username should not be that dark and be more visible. But I think it is using the Link Colour variable from the theme colours and we shouldn't be changing that. A couple of things that can be tried to make it better -

Increase the opacity of the Username to 1 (currently it seems to be 0.56)
or
Use a different theme colour variable for the Username, I played around with it and the Center Channel Text colour variable works best. Additionally, it might be a good idea to wrap the Username in a <strong> tag so the font-weight becomes bold.

@djanda97

mattermod · 2021-03-27T01:00:58Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

hanzei · 2021-03-29T11:55:58Z

Removing myself from the reviewers list util the UX questions are resolved.

djanda97 · 2021-03-30T01:49:49Z

I tested out what the styling looks like when the username is wrapped in an anchor tag, and It looks like the username is a bit dark. I'm wondering if we can make it so that specific word is not dark, while the words around it are dark as they are now. @abhijit-singh Do you have any thoughts on this?

@mickmister I agree that the username should not be that dark and be more visible. But I think it is using the Link Colour variable from the theme colours and we shouldn't be changing that. A couple of things that can be tried to make it better -

Increase the opacity of the Username to 1 (currently it seems to be 0.56)

or

Use a different theme colour variable for the Username, I played around with it and the Center Channel Text colour variable works best. Additionally, it might be a good idea to wrap the Username in a <strong> tag so the font-weight becomes bold.

@djanda97

Thanks for the feedback @abhijit-singh. It might be a while until I can find some time to work on this PR. Once I do, I will try implementing the solutions you listed above.

adithyaakrishna · 2021-10-25T20:06:49Z

@djanda97 Could you please lmk if there's any update on this? If not can I take this up?

djanda97 · 2021-10-30T02:53:18Z

Hi @adithyaakrishna, I haven't made any progress on this PR, so feel free to pick it up!

mickmister · 2021-11-03T04:48:38Z

@adithyaakrishna Would you like to take up this ticket? You can clone this PR's code using the hub command:

git clone [email protected]:mattermost/mattermost-plugin-github.git

# or set the url of origin if you already have this checked out

git remote set-url origin [email protected]:mattermost/mattermost-plugin-github.git
git remote add adithyaakrishna [email protected]:adithyaakrishna/mattermost-plugin-github.git

Then you can run hub pr checkout 424 to check out the correct branch used in this PR. Then you can base a branch off of this branch and push to your own fork, and create a new PR from your fork.

hanzei · 2021-11-15T11:20:42Z

I'm going to close this PR as @adithyaakrishna is working on a new one. Still, thanks @djanda97 for working on this!

djanda97 added 2 commits March 7, 2021 15:52

Made GitHub username clickable

19f615b

Removed quotes

812c4dd

djanda97 requested a review from hanzei as a code owner March 7, 2021 23:59

mickmister suggested changes Mar 8, 2021

View reviewed changes

hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Mar 8, 2021

hanzei requested a review from abhijit-singh March 9, 2021 11:44

hanzei added the 1: UX Review Requires review by a UX Designer label Mar 9, 2021

Construct URL inline; add JSX expression container

1a174f3

mattermod added the Lifecycle/1:stale label Mar 27, 2021

hanzei removed their request for review March 29, 2021 11:55

hanzei removed the request for review from abhijit-singh August 20, 2021 08:48

hanzei added Work In Progress Not yet ready for review and removed 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester 1: UX Review Requires review by a UX Designer labels Oct 1, 2021

hanzei added 3: QA Review Requires review by a QA tester and removed Lifecycle/1:stale labels Nov 15, 2021

hanzei added Lifecycle/3:orphaned and removed Work In Progress Not yet ready for review 3: QA Review Requires review by a QA tester labels Nov 15, 2021

hanzei closed this Nov 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Made GitHub username clickable #424

Made GitHub username clickable #424

djanda97 commented Mar 7, 2021

mattermod commented Mar 7, 2021

djanda97 commented Mar 8, 2021

mickmister left a comment

mickmister Mar 8, 2021 •

edited

Loading

djanda97 Mar 8, 2021

mickmister Mar 8, 2021

mickmister Mar 8, 2021

djanda97 Mar 8, 2021

mickmister commented Mar 8, 2021

mickmister commented Mar 8, 2021

abhijit-singh commented Mar 16, 2021

mattermod commented Mar 27, 2021

hanzei commented Mar 29, 2021

djanda97 commented Mar 30, 2021

adithyaakrishna commented Oct 25, 2021

djanda97 commented Oct 30, 2021 •

edited

Loading

mickmister commented Nov 3, 2021

hanzei commented Nov 15, 2021

-                    {userName && <a href={userProfile}> by {userName}</a>}
+                    {userName && (
+                        <>
+                            {' by '}
+                            <a href={userProfile}>{userName}</a>
+                        </>
+                    )}

Made GitHub username clickable #424

Made GitHub username clickable #424

Conversation

djanda97 commented Mar 7, 2021

Summary

Ticket Link

mattermod commented Mar 7, 2021

djanda97 commented Mar 8, 2021

mickmister left a comment

Choose a reason for hiding this comment

mickmister Mar 8, 2021 • edited Loading

Choose a reason for hiding this comment

djanda97 Mar 8, 2021

Choose a reason for hiding this comment

mickmister Mar 8, 2021

Choose a reason for hiding this comment

mickmister Mar 8, 2021

Choose a reason for hiding this comment

djanda97 Mar 8, 2021

Choose a reason for hiding this comment

mickmister commented Mar 8, 2021

mickmister commented Mar 8, 2021

abhijit-singh commented Mar 16, 2021

mattermod commented Mar 27, 2021

hanzei commented Mar 29, 2021

djanda97 commented Mar 30, 2021

adithyaakrishna commented Oct 25, 2021

djanda97 commented Oct 30, 2021 • edited Loading

mickmister commented Nov 3, 2021

hanzei commented Nov 15, 2021

mickmister Mar 8, 2021 •

edited

Loading

djanda97 commented Oct 30, 2021 •

edited

Loading