Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add link to Github commit #63

Merged
merged 5 commits into from
Feb 10, 2019
Merged

Add link to Github commit #63

merged 5 commits into from
Feb 10, 2019

Conversation

shayanb
Copy link
Contributor

@shayanb shayanb commented Feb 8, 2019

Add a hyperlink on the commit date to open the Github commit in a new page.

@pomber
Copy link
Owner

pomber commented Feb 9, 2019

Thanks, I like it. I prefer to show the link only for the active commit to avoid accidentally clicking on the link when trying to change the commit. You can use isActive to choose what to render.
Also, can yo remove the extra space {" "} at the end of line 45?

@pomber
Copy link
Owner

pomber commented Feb 9, 2019

One more thing, check if there is a commitUrl: isActive && commit.commitUrl

@shayanb
Copy link
Contributor Author

shayanb commented Feb 10, 2019

@pomber thanks, made the changes. Sorry if my method is strange, I'm not really a React dev :)

Copy link
Owner

@pomber pomber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem. You are almost there.

Copy link
Contributor Author

@shayanb shayanb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 62104a0 .

@pomber
Copy link
Owner

pomber commented Feb 10, 2019

Thanks!

@pomber pomber merged commit 06fee9e into pomber:master Feb 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants