Add GitHub components for Repos, Issues, PullRequests #2367

menghif · 2021-10-18T22:13:23Z

Issue This PR Addresses

Type of Change

Bugfix: Change which fixes an issue
New Feature: Change which adds functionality
Documentation Update: Change which improves documentation
UI: Change which improves UI

Description

This PR adds a way to display links for all GitHub repos, issues and pull requests that are linked throughout a post. This information is displayed on the right sidebar underneath the blog post's date for easy visibility.

If there is no repo URL in the post, but there is at least one issue or PR, the repo URL will be taken from that issue or PR.

Screenshots:

Checklist

Quality: This PR builds and passes our npm test and works locally
Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

humphd · 2021-10-18T22:20:51Z

Let's reduce vertical space between rows:

menghif · 2021-10-18T22:23:51Z

How do I fix this?

Screen.Recording.2021-10-18.at.6.19.30.PM.mov

humphd · 2021-10-18T22:24:56Z

How do I fix this?

I can't see what you're talking about

humphd · 2021-10-18T22:26:17Z

@Andrewnt219, can you review this too please?

menghif · 2021-10-18T22:26:22Z

The github info goes over the title and date when I scroll to the bottom of the post.

menghif · 2021-10-18T22:41:13Z

Also I just realized I swapped the icons for Repos and Pull Requests 😬 Fixed now!

src/web/src/components/Posts/GitHubInfo.tsx

humphd

I'm loving this. A few things I notice on first read.

src/web/src/components/Posts/GitHubInfo.tsx

src/web/src/components/Posts/Issues.tsx

manekenpix · 2021-10-19T00:38:25Z

Issue with long names (check the repo)

humphd · 2021-10-19T00:40:06Z

Issue with long names (check the repo)

I noticed this too. One question I had: should we only show the repo name and not the owner? So Seneca-CDOT/telescope might show as telescope unless you hover and the title can show the whole thing? Not sure if that's good or bad, but it would help with this a bit.

Andrewnt219 · 2021-10-19T00:45:25Z

Issue with long names (check the repo)

This may be a problem with Safari. Doesn't happen on Firefox or Chrome on Window.

Add title attrbute to Issues and PRs Simplify regex to get Issue/PR number

Add title attrbute to Repo: "user/repo" Remove user from repo name displayed

menghif · 2021-10-19T02:55:15Z

Issue with long names (check the repo)

I noticed this too. One question I had: should we only show the repo name and not the owner? So Seneca-CDOT/telescope might show as telescope unless you hover and the title can show the whole thing? Not sure if that's good or bad, but it would help with this a bit.

I tried this and it looks better in my opinion. You can check the last deployment:
https://telescope-jt4xjjrp2-humphd.vercel.app/

humphd

I left some more feedback. Feel free to break the work up a bit, and file follow-up issues that we can work on vs. feeling like we have to get this perfect before we can merge. I'd be happy to take it as-is once we get the last few edges sanded down.

src/web/src/components/Posts/GitHubInfo.tsx

src/web/src/components/Posts/Issues.tsx

src/web/src/components/Posts/PullRequests.tsx

Andrewnt219 · 2021-10-19T15:52:13Z

Also notice this. They're forked repos.

humphd · 2021-10-20T13:28:16Z

I filed #2370, #2371, #2372, and #2373 for follow-up. @menghif, @Andrewnt219: anything else we're not going to do in this PR, please file as new issues that we can work on later once this goes in.

The fixes above based on @DukeManh's review could happen here I think, plus any other code changes based on subsequent reviews.

HyperTHD · 2021-10-20T23:04:07Z

I just wanna confirm that the repo looking buggy when the name is long is also appearing for me on Google Chrome.

Just something to consider if and when this does get merged, since the css used here never had this idea in mind

humphd · 2021-10-21T00:00:40Z

@HyperTHD I can't reproduce, Is it a line-height issue? Yes, we need to file an issue on this one too.

HyperTHD · 2021-10-21T02:28:00Z

@HyperTHD I can't reproduce, Is it a line-height issue? Yes, we need to file an issue on this one too.

I suspect that is the issue. Unless I'm mistaken, this code should be the issue causing this.

https://github.com/Seneca-CDOT/telescope/pull/2367/files#diff-52847b29df0db31f168773ddd4e97cc9828a6aac76a638d1096f07f751b34bc6R29

Use h2 instead of h1 for Repo, Issue and PR title. Wrap extractGitHubUrlsFromPost into react's useMemo.

humphd

Really close, a few more things.

src/web/src/components/Posts/Post.tsx

humphd · 2021-10-21T18:07:23Z

src/web/src/components/Posts/Repos.tsx

+);
+
+const getUserName = (repo: string) => {
+  return repo.replace(/\/([^\/]+).*/, '$1');


Get rid of { return, you don't need it.

I got rid of the function all together since we don't need it for now.

humphd · 2021-10-21T18:08:09Z

src/web/src/components/Posts/Repos.tsx

+      {repoUrls.map((repo) => (
+        <p key={repo} className={classes.repo}>
+          <a
+            href={`https://github.com${repo}`}


Does the repo have a leading / or not? In line 57 you assume it does, but in line 60 you assume it doesn't. We need to be consistent.

I would suggest it should not have a / at all.

I removed the leading / and made the appropriate changes.

@humphd @menghif to be consistent, best to keep the leading slash in repo URLs. URL.pathname returns a string with a leading slash, so issues, commits, pull requests all have a leading slash.

This can be changed in #2374 if necessary.

humphd

Love it. Can't wait to start using this!

humphd · 2021-10-21T20:24:32Z

Filed #2380 to do a bit of reorg on these files later on.

humphd · 2021-10-21T22:08:19Z

This needs a rebase, but since it's got two authors, I'll do it for you and merge.

humphd · 2021-10-21T22:28:21Z

Rebased and merged in bce0b21. Great job everyone!

Add GitHub components for Repos, Issues, PullRequests

1d6bf6b

vercel bot deployed to Preview October 18, 2021 22:13 View deployment

humphd requested review from DukeManh, manekenpix, PedroFonsecaDEV and HyperTHD October 18, 2021 22:25

menghif added 2 commits October 18, 2021 18:43

fix icons for Repos and Pull Requests

707a5ca

reduce spacing between github repos list

edd7f63

vercel bot deployed to Preview October 18, 2021 22:54 View deployment

Andrewnt219 reviewed Oct 18, 2021

View reviewed changes

src/web/src/components/Posts/GitHubInfo.tsx Outdated Show resolved Hide resolved

src/web/src/components/Posts/GitHubInfo.tsx Outdated Show resolved Hide resolved

humphd requested changes Oct 19, 2021

View reviewed changes

menghif added 3 commits October 18, 2021 21:31

fix PullRequests.tsx lowercase filename

b7a76db

Open Issues and PRs in new window.

d98aff4

Add title attrbute to Issues and PRs Simplify regex to get Issue/PR number

Open Repo in new window.

8d72bd5

Add title attrbute to Repo: "user/repo" Remove user from repo name displayed

vercel bot deployed to Preview October 19, 2021 02:46 View deployment

Andrewnt219 mentioned this pull request Oct 19, 2021

Fix #2360: refactor: change filterGitHubUrls() menghif/telescope#1

Merged

8 tasks

refactor: change filterGitHubUrls()

9ac9f99

humphd requested changes Oct 19, 2021

View reviewed changes

replace array with Set

833e6ed

humphd mentioned this pull request Oct 20, 2021

Include more details about Issue and Pull Requests in title attribute #2374

Closed

Use JSX conditional syntax in GitHubInfo.

920977f

Use h2 instead of h1 for Repo, Issue and PR title. Wrap extractGitHubUrlsFromPost into react's useMemo.

menghif dismissed Andrewnt219’s stale review via 920977f October 21, 2021 17:53

vercel bot deployed to Preview October 21, 2021 17:53 View deployment

menghif requested review from DukeManh and humphd October 21, 2021 17:58

humphd requested changes Oct 21, 2021

View reviewed changes

Remove leading / from repoUrl. Remove getUserName

c4ed6c8

vercel bot deployed to Preview October 21, 2021 20:06 View deployment

Andrewnt219 mentioned this pull request Oct 21, 2021

Fix 2372: Add commit to Github sidebar #2376

Merged

8 tasks

humphd mentioned this pull request Oct 21, 2021

Get extractedGitHubUrls to work with conditional rendering #2378

Closed

humphd previously approved these changes Oct 21, 2021

View reviewed changes

Andrewnt219 mentioned this pull request Oct 21, 2021

Remove duplicate code between Issues, Commits, Repos, PullRequest #2379

Closed

humphd mentioned this pull request Oct 21, 2021

Reorganize GitHubInfo components into a directory #2380

Closed

use react Conditional Rendering

1b32de0

menghif dismissed humphd’s stale review via 1b32de0 October 21, 2021 20:49

vercel bot deployed to Preview October 21, 2021 20:49 View deployment

DukeManh approved these changes Oct 21, 2021

View reviewed changes

humphd approved these changes Oct 21, 2021

View reviewed changes

humphd closed this Oct 21, 2021

humphd added the hacktoberfest-accepted label Oct 21, 2021

menghif deleted the issue-2360 branch December 8, 2021 01:46

JiaHua-Zou mentioned this pull request Feb 2, 2022

Reorganize GithubInfo componment #2806

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GitHub components for Repos, Issues, PullRequests #2367

Add GitHub components for Repos, Issues, PullRequests #2367

menghif commented Oct 18, 2021

humphd commented Oct 18, 2021

menghif commented Oct 18, 2021

humphd commented Oct 18, 2021

humphd commented Oct 18, 2021

menghif commented Oct 18, 2021

menghif commented Oct 18, 2021 •

edited

Loading

humphd left a comment

manekenpix commented Oct 19, 2021

humphd commented Oct 19, 2021

Andrewnt219 commented Oct 19, 2021

menghif commented Oct 19, 2021

humphd left a comment

Andrewnt219 commented Oct 19, 2021 •

edited

Loading

humphd commented Oct 20, 2021

HyperTHD commented Oct 20, 2021

humphd commented Oct 21, 2021

HyperTHD commented Oct 21, 2021 •

edited

Loading

humphd left a comment

humphd Oct 21, 2021

menghif Oct 21, 2021

humphd Oct 21, 2021

humphd Oct 21, 2021

menghif Oct 21, 2021

Andrewnt219 Oct 21, 2021

menghif Oct 21, 2021

humphd left a comment

humphd commented Oct 21, 2021

humphd commented Oct 21, 2021

humphd commented Oct 21, 2021

Add GitHub components for Repos, Issues, PullRequests #2367

Add GitHub components for Repos, Issues, PullRequests #2367

Conversation

menghif commented Oct 18, 2021

Issue This PR Addresses

Type of Change

Description

Checklist

humphd commented Oct 18, 2021

menghif commented Oct 18, 2021

humphd commented Oct 18, 2021

humphd commented Oct 18, 2021

menghif commented Oct 18, 2021

menghif commented Oct 18, 2021 • edited Loading

humphd left a comment

Choose a reason for hiding this comment

manekenpix commented Oct 19, 2021

humphd commented Oct 19, 2021

Andrewnt219 commented Oct 19, 2021

menghif commented Oct 19, 2021

humphd left a comment

Choose a reason for hiding this comment

Andrewnt219 commented Oct 19, 2021 • edited Loading

humphd commented Oct 20, 2021

HyperTHD commented Oct 20, 2021

humphd commented Oct 21, 2021

HyperTHD commented Oct 21, 2021 • edited Loading

humphd left a comment

Choose a reason for hiding this comment

humphd Oct 21, 2021

Choose a reason for hiding this comment

menghif Oct 21, 2021

Choose a reason for hiding this comment

humphd Oct 21, 2021

Choose a reason for hiding this comment

humphd Oct 21, 2021

Choose a reason for hiding this comment

menghif Oct 21, 2021

Choose a reason for hiding this comment

Andrewnt219 Oct 21, 2021

Choose a reason for hiding this comment

menghif Oct 21, 2021

Choose a reason for hiding this comment

humphd left a comment

Choose a reason for hiding this comment

humphd commented Oct 21, 2021

humphd commented Oct 21, 2021

humphd commented Oct 21, 2021

menghif commented Oct 18, 2021 •

edited

Loading

Andrewnt219 commented Oct 19, 2021 •

edited

Loading

HyperTHD commented Oct 21, 2021 •

edited

Loading