Skip to content

fix multiline breaking links #681

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

Merged
merged 1 commit into from
May 27, 2025

Conversation

jmusial
Copy link
Contributor

@jmusial jmusial commented May 22, 2025

Fix multiline breaking blockquote
Fix multiline breaking links underline

Details

Links underline

There is another fix possible
With link text staying vertical-align: middle and just pushing the underline lower.
Not sure which one is prefereable.

Blockquote

Turns out when we have white-space: nowrap contentEditable changes spaces to non-breaking (ASCII 160 vs normal 32). This is the simplest fix. Alternative is to replace them in the event or make ExpensiMark recognize them as spaces. Again, not sure which one is preferable.

Related Issues

#548

Videos

Before

LM_multiline_before.mov

After

LM_multiline_after.mov

Manual Tests

Links underline

  1. Open web example app
  2. Toggle multiline off
  3. Links underline show properly

Blockquote

  1. Open web example app
  2. Remove all content
  3. Toggle multiline off
  4. Start writing a blockquote (> )
  5. It works same as with multiline on

Linked PRs

Copy link

github-actions bot commented May 22, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@jmusial
Copy link
Contributor Author

jmusial commented May 22, 2025

I have read the CLA Document and I hereby sign the CLA

Copy link
Collaborator

@Skalakid Skalakid left a comment

Choose a reason for hiding this comment

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

Nice LGTM :D

@jmusial jmusial marked this pull request as ready for review May 23, 2025 15:26
fix multiline breaking blockquote
@jmusial jmusial force-pushed the fix/multiline-issues-on-web branch from 37f898a to a3f9ca7 Compare May 23, 2025 15:45
CLABotify added a commit to Expensify/CLA that referenced this pull request May 23, 2025
@tomekzaw
Copy link
Collaborator

tomekzaw commented May 25, 2025

Turns out when we have white-space: nowrap contentEditable changes spaces to non-breaking (ASCII 160 vs normal 32).

This is a really cool finding. Does this give us some more opportunities to simplify our code in other places as well?

Also, could you please upload some screen recordings showing how it worked before and how it works after the changes?

@jmusial
Copy link
Contributor Author

jmusial commented May 27, 2025

@tomekzaw added vids.

@Skalakid tested with Expensify/App - seems fine.

Regarding simplifying codebase AFAIK no, but I'm still fresh to the project so might not know sth :)

@Skalakid Skalakid merged commit 1626f97 into Expensify:main May 27, 2025
5 checks passed
@os-botify
Copy link
Contributor

os-botify bot commented May 27, 2025

🚀 Published to npm in 0.1.279 🎉

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.

3 participants