LBRYCommunity/lbry-desktop

Fork 0

Add support for Clickable channel names on Explore page headings (#1820) #2023

Merged

chrisza4 merged 3 commits from clickable-channel into master

2018-10-11 22:26:54 +02:00

chrisza4 commented

2018-10-10 16:21:55 +02:00

(Migrated from github.com)

Based on this issue:
https://github.com/lbryio/lbry-desktop/issues/1820

I am new to lbryio, but I assume that channel name can be written in two ways:

@CryptoCandor#9152f3b054f692076a6882d1b58a30e8781cc8e6
Crypto Candor | @CryptoCandor#9152f3b054f692076a6882d1b58a30e8781cc8e6

I assume that channel name would always start with @. So I cut the string by @ signed, check if it has link, and pass that to categorylink. This solution maintain backward compatibility.

I do not know where should I put the new function, but I do not want to clutter the render method. So I separate out as getCategoryLinkPartByCategory in this component.

Based on this issue: https://github.com/lbryio/lbry-desktop/issues/1820 I am new to lbryio, but I assume that channel name can be written in two ways: `@CryptoCandor#9152f3b054f692076a6882d1b58a30e8781cc8e6` `Crypto Candor | @CryptoCandor#9152f3b054f692076a6882d1b58a30e8781cc8e6` I assume that channel name would always start with `@`. So I cut the string by `@` signed, check if it has link, and pass that to categorylink. This solution maintain backward compatibility. I do not know where should I put the new function, but I do not want to clutter the render method. So I separate out as `getCategoryLinkPartByCategory` in this component.

🎉 1

neb-b (Migrated from github.com) reviewed 2018-10-10 16:21:55 +02:00

neb-b commented

2018-10-10 16:55:57 +02:00

(Migrated from github.com)

Awesome! Thanks for the contribution 🙂

I think this should work. I'll give it a test in a bit.

Awesome! Thanks for the contribution 🙂 I think this should work. I'll give it a test in a bit.

neb-b commented

2018-10-10 16:59:01 +02:00

(Migrated from github.com)

I think we will also need a function for stripping the claim id off of the category title so that we aren't displaying the claim id for every line. This could be similar to how you generate the categoryLink

category={this.trimClaimIdFromCategory} (or something similar)

👍 1

tzarebczan commented

2018-10-10 17:01:13 +02:00

(Migrated from github.com)

Hey @chrisza4, thanks for checking out LBRY and the contribution! If you haven't already, please make sure to check out our contributing guide.

Can we send you LBC in appreciation for your work?

Hey @chrisza4, thanks for checking out LBRY and the contribution! If you haven't already, please make sure to check out our [contributing guide](https://github.com/lbryio/lbry-desktop/blob/master/CONTRIBUTING.md). Can we send you [LBC in appreciation for your work](https://lbry.io/faq/appreciation)?

chrisza4 commented

2018-10-10 17:05:53 +02:00

(Migrated from github.com)

@seanyesmunt That is correct. Let's make sure we always trim claim id from category before send props to CategoryList component.

@tzarebczan Yes you can. Do you need any information from me? My email is chakrit.lj@gmail.com

@seanyesmunt That is correct. Let's make sure we always trim claim id from category before send props to `CategoryList` component. @tzarebczan Yes you can. Do you need any information from me? My email is chakrit.lj@gmail.com

tzarebczan commented

2018-10-10 17:26:55 +02:00

(Migrated from github.com)

@chrisza4 please check out the FAQ that I linked, we'll need you to reach out to us. Also, please include a time estimate.

chrisza4 commented

2018-10-10 18:38:21 +02:00

(Migrated from github.com)

Thanks @tzarebczan . I will reach out to you once PR get merged.

neb-b commented

2018-10-11 07:59:55 +02:00

(Migrated from github.com)

Tested and it's working good!

@chrisza4 Can you add a changelog update? Then it's ready to merge.

Tested and it's working good! @chrisza4 Can you add a [changelog](https://github.com/lbryio/lbry-desktop/blob/master/CHANGELOG.md) update? Then it's ready to merge.

chrisza4 commented

2018-10-11 17:37:17 +02:00

(Migrated from github.com)

Changelog updated

tzarebczan commented

2018-10-11 20:22:24 +02:00

(Migrated from github.com)

woohoo @chrisza4 , nice job getting through your first PR! Don't forget to reach out once it's merged :)

neb-b commented

2018-10-11 20:45:33 +02:00

(Migrated from github.com)

Awesome! I’ll merge this once I’m back at my computer