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

Fix LinkedIn URL special character formatting #7249

Merged

Conversation

ad-elias
Copy link
Contributor

Before:
link-formatting-before

After:
linkedin-formatting-after

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request addresses the formatting of LinkedIn URLs containing special characters, improving the display of profile identifiers.

  • Modified packages/twenty-front/src/utils/getDisplayValueByUrlType.ts to use decodeURIComponent() for LinkedIn URLs
  • Enhances readability of LinkedIn profile names and company identifiers with encoded characters
  • Affects components using getDisplayValueByUrlType(), such as LinkDisplay, URLDisplay, and SocialLink
  • Improves user experience by showing more accurate LinkedIn URL representations
  • Visual improvement demonstrated in before/after screenshots in the pull request description

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@@ -16,7 +16,7 @@ export const getDisplayValueByUrlType = ({
/(?:https?:\/\/)?(?:www.)?linkedin.com\/(?:in|company|school)\/(.*)/,
);
if (isDefined(matches?.[1])) {
return matches?.[1];
return decodeURIComponent(matches?.[1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM! @bosiraphael

Copy link
Contributor

@bosiraphael bosiraphael left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @ad-elias for your contribution :)

@bosiraphael bosiraphael merged commit c9e882f into twentyhq:main Sep 25, 2024
4 of 10 checks passed
Copy link

Thanks @ad-elias for your contribution!
This marks your 4th PR on the repo. You're top 8% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

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

Successfully merging this pull request may close these issues.

3 participants