Skip to content

Conversation

@Skalakid
Copy link
Contributor

@Skalakid Skalakid commented Oct 6, 2025

Details

This is a follow-up PR to the previous fix for cursor positioning in inline code markdown. Here, I introduced a mechanism that marks opening and closing syntax, and based on that, it extends the range before opening syntax and the range after closing syntax. Thanks to that, styles are continuous

Related Issues

Expensify/App#69493

Manual Tests

  1. Open the example app
  2. Paste the following text:
_test🚀_
~🚀test~
~_tes🚀t_~
_`test🚀`_
~`🚀test`~
~_`tes🚀t`_~
  1. Verify if strikethrough or italic styles aren't applied to any emoji
  2. Verify I styles are properly applied on opening and closing syntaxes of inline code markdowns

Linked PRs

#729

jmusial
jmusial previously approved these changes Oct 6, 2025
Copy link
Collaborator

@jmusial jmusial left a comment

Choose a reason for hiding this comment

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

LGTM :feelsgood:

* This includes emojis and syntaxes of inline code blocks.
*/
function getRangesToExcludeFormatting(ranges: MarkdownRange[]) {
function getRangesToExcludeFormatting(ranges: MarkdownRange[]): ExtendedMarkdownRange[] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

NAB but i think this should do and you can get rid of #71 then

Suggested change
function getRangesToExcludeFormatting(ranges: MarkdownRange[]): ExtendedMarkdownRange[] {
function getRangesToExcludeFormatting(ranges: ExtendedMarkdownRange[]): ExtendedMarkdownRange[] {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I do that, I will get eslintno-param-reassign. So we need to keep #71. Removing as doesn't seem to me to be worthy, because this function should transform MarkdownRange to ExtendedMarkdownRange. It's more logical to me in that way

import {expect} from '@jest/globals';
import type {MarkdownRange} from '../commonTypes';
import parseExpensiMark from '../parseExpensiMark';
import type {ExtendedMarkdownRange} from '../rangeUtils';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a huge fan of ExtendedMarkdownRange, why do we need this? Why can't we just extend the existing MarkdownRange type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, we can 🤔 Changed that in 0169cdc

@jmusial jmusial merged commit ef19e3d into Expensify:main Oct 8, 2025
5 checks passed
@os-botify
Copy link
Contributor

os-botify bot commented Oct 8, 2025

🚀 Published to npm in 0.1.307 🎉

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