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

feat: Enable links in bio #10337

Merged
merged 20 commits into from
Jun 19, 2024
Merged

feat: Enable links in bio #10337

merged 20 commits into from
Jun 19, 2024

Conversation

hassnian
Copy link
Contributor

@hassnian hassnian commented May 23, 2024

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Needs QA check

  • @kodadot/qa-guild please review

Did your issue had any of the "$" label on it?

  • Fill up your DOT address: Payout

Screenshot 📸

  • My fix has changed something on UI;

CleanShot 2024-05-29 at 09 59 06@2x

Copy link

netlify bot commented May 23, 2024

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit a5869e6
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/66729a90707cdc0008a43d8f
😎 Deploy Preview https://deploy-preview-10337--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hassnian hassnian marked this pull request as ready for review May 29, 2024 04:58
@hassnian hassnian requested a review from a team as a code owner May 29, 2024 04:58
@hassnian hassnian requested review from preschian and Jarsen136 and removed request for a team May 29, 2024 04:58
@prury
Copy link
Member

prury commented May 29, 2024

can we make it not affect markdown tags?

image

@prury prury added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label May 29, 2024
@hassnian
Copy link
Contributor Author

hassnian commented May 30, 2024

can we make it not affect markdown tags?

pls try again

@prury prury added S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked and removed S-changes-requested-🤞 PR is almost good to go, just some fine tunning labels May 30, 2024
Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

TL;DR

parsing desc and checking for /channle (eg. /koda) or @user (e.g @vikiival)

cc @exezbcz I think it should be done on the create profile level, it would break markdown links imo

Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

as pointed in comment above

cc @daiagi

@exezbcz
Copy link
Member

exezbcz commented Jun 5, 2024

@vikiival how?

@vikiival
Copy link
Member

vikiival commented Jun 5, 2024

@vikiival how?

This PR

https://deploy-preview-10337--koda-canary.netlify.app/ahp/u/14BZFbYEGoWWPjbbZUdLZ1TqDtdXvdeCy3R3t4QXqJmS91Dx

Screenshot 2024-06-05 at 12 51 38

Prod:

Screenshot 2024-06-05 at 12 52 21

@vikiival
Copy link
Member

vikiival commented Jun 7, 2024

TL;DR of your changes?

@vikiival vikiival requested a review from Jarsen136 June 7, 2024 11:05
@prury
Copy link
Member

prury commented Jun 10, 2024

TL;DR of your changes?

Links are now added on profile save

@prury can you test again, this should be fixed , thanks

seems good now

@vikiival
Copy link
Member

Screenshot 2024-06-11 at 11 33 17

@vikiival
Copy link
Member

Works as intented.
But I have a problem:

I use create profile (manual), inserted fan of /koda it was automatically translated to warpcast link.

My question is: Should it be enabled only in case when you use import from farcaster ?

cc @exezbcz

@vikiival
Copy link
Member

Screenshot 2024-06-11 at 11 41 21

also thinking why you do not translate it to markdown links? rather than html?

<a href="https://warpcast.com/~/channel/koda" target="_blank" rel="nofollow noopener noreferrer">/koda</a>

vs

[/koda](https://warpcast.com/~/channel/koda)

@exezbcz
Copy link
Member

exezbcz commented Jun 11, 2024

My question is: Should it be enabled only in case when you use import from farcaster ?

/koda should be translated only when importing from farcaster

as /koda can mean something else on koda itself in the future

Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

as per exez's comment above

@vikiival vikiival removed the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label Jun 12, 2024
@hassnian
Copy link
Contributor Author

also thinking why you do not translate it to markdown links? rather than html?

@vikiival so they open in a new tab , can change it if it's not important.

@hassnian
Copy link
Contributor Author

My question is: Should it be enabled only in case when you use import from farcaster ?

/koda should be translated only when importing from farcaster

as /koda can mean something else on koda itself in the future

fixed

@vikiival
Copy link
Member

also thinking why you do not translate it to markdown links? rather than html?

@vikiival so they open in a new tab , can change it if it's not important.

Every link on profiles are markdown based. I prefer to have it unified

@hassnian
Copy link
Contributor Author

Every link on profiles are markdown based. I prefer to have it unified

done

@hassnian hassnian requested a review from vikiival June 13, 2024 13:48
Copy link

sonarcloud bot commented Jun 16, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarCloud

@prury
Copy link
Member

prury commented Jun 17, 2024

@hassnian can you check profile creation trough farcaster using this PR? i tried testing here and it won't work
on canary it goes normally.

Copy link

codeclimate bot commented Jun 19, 2024

Code Climate has analyzed commit a5869e6 and detected 0 issues on this pull request.

View more on Code Climate.

@vikiival
Copy link
Member

@hassnian can you check profile creation trough farcaster using this PR? i tried testing here and it won't work on canary it goes normally.

cc @hassnian

@hassnian
Copy link
Contributor Author

hassnian commented Jun 19, 2024

@hassnian can you check profile creation trough farcaster using this PR? i tried testing here and it won't work on canary it goes normally.

using new worker: same issues as #10485 (comment)
using older version of the worker: works fine ✅

CleanShot.2024-06-19.at.14.07.24.mp4

@vikiival vikiival added this pull request to the merge queue Jun 19, 2024
Merged via the queue into kodadot:main with commit c14b519 Jun 19, 2024
18 of 19 checks passed
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.

Enable links in bio
5 participants