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 `highest-rated-comment` feature #2108

Merged
merged 17 commits into from Jun 4, 2019

Conversation

Projects
None yet
2 participants
@lubien
Copy link
Contributor

commented Jun 1, 2019

lubien added some commits Jun 1, 2019

@bfred-it
Copy link
Collaborator

left a comment

Thanks for the PR! Got a few changes 😅

Show resolved Hide resolved source/features/highest-rated-comment.css Outdated
Show resolved Hide resolved source/features/highest-rated-comment.css Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated

@bfred-it bfred-it added the enhancement label Jun 1, 2019

@lubien

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2019

I'm on it

@lubien

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2019

Done

@bfred-it
Copy link
Collaborator

left a comment

Can you commit to maintain this feature going forward? For example, in case GitHub changes something that breaks it. We already have way too many features to maintain, so we need to find a way to make this sustainable.

If so, can you add a line to the CODEOWNERS file?

Show resolved Hide resolved source/libs/icons.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated

@bfred-it bfred-it force-pushed the sindresorhus:master branch from 5df5c5f to 5f1a966 Jun 1, 2019

lubien added some commits Jun 1, 2019

Updates on highest-rated-comments
* Simplify template.
* Update CSS to fit the template changes.
* Update init signature.
* Remove $ from DOM variables.
* Minor implementation changes.
* Correct arrowDown SVG template.
Add @lubien to CODEOWNERS
Feature: highest-rated-comment.tsx
@lubien

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2019

Implemented and added to CODEOWNERS ;)

Update rgh-highest-rated-comment CSS
We need to force just the color since the original comment original CSS
takes precedence unless we do this.
@bfred-it
Copy link
Collaborator

left a comment

Probably the last pass 😃

Show resolved Hide resolved source/features/highest-rated-comment.css Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Show resolved Hide resolved source/features/highest-rated-comment.tsx Outdated
Simplify rgh-highest-rated-comment
* Oneliner CSS border.
* JSX render function directly in init.
* Remove Prop type.
* Simplify JSX.
@lubien

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2019

Done ;)

lubien and others added some commits Jun 1, 2019

@bfred-it bfred-it self-assigned this Jun 2, 2019

@bfred-it

This comment has been minimized.

Copy link
Collaborator

commented Jun 2, 2019

New screenshot:

@bfred-it bfred-it removed their assignment Jun 2, 2019

@bfred-it

This comment has been minimized.

Copy link
Collaborator

commented Jun 2, 2019

Can you find more threads where this can be tested?

@lubien

This comment has been minimized.

Copy link
Contributor Author

commented Jun 2, 2019

I tried to search for cases on the 5th+ comment but had no luck over some huge repos I went through. Most would be the first one.

About the changes I can tackle monday if it's okay to you. Today I have to deal with some things first :)

@bfred-it bfred-it changed the title Add highest-rated-comment feature Add `highest-rated-comment` feature Jun 4, 2019

@bfred-it bfred-it merged commit 47f3c74 into sindresorhus:master Jun 4, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@bfred-it

This comment has been minimized.

Copy link
Collaborator

commented Jun 4, 2019

This is great and will definitely super useful! Thank you @lubien! 🥇

@lubien

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

Oh. I was gonna do it when I came back home. Thanks for your help

Glad to work with you and see you next time

@lubien lubien deleted the lubien:highest-rated-comment branch Jun 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.