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: link overflow truncate #463

Merged
merged 6 commits into from
Jul 7, 2022
Merged

fix: link overflow truncate #463

merged 6 commits into from
Jul 7, 2022

Conversation

Vyvy-vi
Copy link
Collaborator

@Vyvy-vi Vyvy-vi commented Jun 12, 2022

Fixes #457
Fixes #465
this truncates long URLs to 50-character long hyperlinks.
and takes care of symbols inside hyperlinks.

Screenshot 2022-06-12 at 7 02 36 AM
Screenshot 2022-07-04 at 1 54 37 PM
Screenshot 2022-07-04 at 1 52 10 PM

mattyg
mattyg previously requested changes Jun 12, 2022
Copy link
Collaborator

@mattyg mattyg left a comment

Choose a reason for hiding this comment

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

awesome thanks vy!

couple issues:

  • If the viewport is too small, a 50 char URL will still overflow the content area. We need to add a CSS rules to break words if necessary. See the screenshot below. I think this is the CSS rule that resolves it: https://tailwindcss.com/docs/word-break#break-words

image

  • lets consolidate this logic to render url strings to html links outside of the Praise component. We should be able to keep all the relevant logic in either components/MarkdownText.tsx or utils/parser.ts (we might want to consolidate them too).

  • We have some redundant duplication of logic: in components/MarkdownText.tsx we use the linkifyHtml() function to convet url strings to html links. In this PR we're converting them twice (again in in parser.tsx), and then we're truncating any urls in Praise.tsx. Can we consolidate this into one implementation?

  • If we want to keep using linkifyHtml(), it has a built-in option for handling truncation. See the option "format" on https://linkify.js.org/docs/options.html - might be simpler code but I don't have a strong preference.

  • While looking at this I noticed another bug that it probably makes sense to address in the same PR, since they related to the same code. The bug is that if a url contains any markdown symbols, the html link rendering is broken. See Praise reasons containing URLs that including certain symbols are not rendered properly #465

@Vyvy-vi Vyvy-vi requested a review from mattyg July 4, 2022 08:38
@Vyvy-vi Vyvy-vi dismissed mattyg’s stale review July 4, 2022 08:40

outdated review

@Vyvy-vi Vyvy-vi requested review from nebs-dev and removed request for mattyg July 4, 2022 19:13
@Vyvy-vi Vyvy-vi changed the base branch from main to development July 4, 2022 19:14
@nebs-dev
Copy link
Contributor

nebs-dev commented Jul 5, 2022

Nice job.. for some reason discord-markdown didn't work for me and that is why i created custom parser, this is even better.

@Vyvy-vi Vyvy-vi merged commit 19157fe into development Jul 7, 2022
@Vyvy-vi Vyvy-vi deleted the fix/link-overflow branch July 7, 2022 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants