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

Enable reaction avatars for teams' discussions #871

Merged

Conversation

lukaszklis
Copy link
Contributor

@lukaszklis lukaszklis commented Dec 13, 2017

I found out today the reaction avatars were not working for the recently launched ”Discussions” feature on GitHub. Unfortunately I do not have a live public example to link directly to test it, however I attach some screenshots before and after the change.

Before:
before

After:
after

@@ -88,3 +88,5 @@ export const isSingleCommit = () => /^commit\/[0-9a-f]{5,40}/.test(getRepoPath()
export const isSingleFile = () => /^blob\//.test(getRepoPath());

export const isTrending = () => location.pathname.startsWith('/trending');

export const isDiscussion = () => /^([^/]+[/][^/]+\/)teams[/][^/]+[^/]+($|[/]$|[/]discussions)/.test(getCleanPathname());
Copy link
Member

Choose a reason for hiding this comment

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

[/] => \/

@@ -88,3 +88,5 @@ export const isSingleCommit = () => /^commit\/[0-9a-f]{5,40}/.test(getRepoPath()
export const isSingleFile = () => /^blob\//.test(getRepoPath());

export const isTrending = () => location.pathname.startsWith('/trending');

export const isDiscussion = () => /^([^/]+\/[^/]+\/)teams\/[^/]+[^/]+($|\/$|\/discussions)/.test(getCleanPathname());
Copy link
Member

Choose a reason for hiding this comment

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

I think \/$| isn't necessary. getCleanPathname() never ends with a trailing slash.

@lukaszklis
Copy link
Contributor Author

lukaszklis commented Dec 13, 2017 via email

@lukaszklis
Copy link
Contributor Author

@bfred-it thanks!

@fregante fregante merged commit 8e4c169 into refined-github:master Dec 14, 2017
@fregante
Copy link
Member

Thanks @lukaszklis!

@lukaszklis lukaszklis deleted the reaction-avatars-for-discussions branch December 14, 2017 10:04
@lukaszklis
Copy link
Contributor Author

lukaszklis commented Dec 14, 2017

@bfred-it A little offtopic question related to the changes requested in the PR: does it make sense to adjust linters to meet some of the requirements whenever people contribute to the repo? It might save maintainers’ priceless time on trivial topics. :)

@fregante
Copy link
Member

You mean custom rules? They'd be great, but then we'd have to also (know how to) code and debug them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants