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 cursor positioning on android on markdown style change #325

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

Skalakid
Copy link
Collaborator

@Skalakid Skalakid commented Apr 22, 2024

Details

Related Issues

Expensify/App#40504

Manual Tests

  1. Modify the MarkdownTextInput component in App. tsx with the following code:
     <MarkdownTextInput
        multiline
        autoCapitalize="none"
        value={value}
        onChangeText={(val) => {
          if (val.includes(':+1:'))r {
            setMarkdownStyle({emoji: {fontSize: 30}});
          }
          setValue(val.replaceAll(':+1:', '👍'));
        }}
        style={styles.input}
        ref={ref}
        markdownStyle={markdownStyle}
        placeholder="Type here..."
        onSelectionChange={(e) => setSelection(e.nativeEvent.selection)}
        selection={selection}
      />
  1. Clear the whole text
  2. Enter 👍
  3. Check if the cursor is positioned correctly (after the emoji)

Linked PRs

@Skalakid Skalakid requested a review from tomekzaw April 22, 2024 12:29
@Skalakid Skalakid self-assigned this Apr 22, 2024
@Skalakid Skalakid requested a review from j-piasecki April 22, 2024 14:38
@Skalakid Skalakid marked this pull request as ready for review April 22, 2024 14:40
@Skalakid Skalakid requested a review from robertKozik April 24, 2024 11:12
@robertKozik
Copy link
Collaborator

@Skalakid Is it testable in example app?

@BartoszGrajdek
Copy link
Collaborator

@Skalakid Is it testable in example app?

Bump 👀

Please add some test steps 🙏🏻

Copy link
Collaborator

@robertKozik robertKozik left a comment

Choose a reason for hiding this comment

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

I've tested on E/App - working like a charm. I didn't find any regressions in example app as well. Changes are quite straightforward so I'll approve it now.

@Skalakid
Copy link
Collaborator Author

Skalakid commented Apr 24, 2024

@BartoszGrajdek @robertKozik Sorry, I forgot to add test steps. The issue is in E/App because we are changing text and styles (for example changing 👍 into an emoji). We can reproduce it in our example app only after app modifying App.tsx file. For example:

      <MarkdownTextInput
        multiline
        autoCapitalize="none"
        value={value}
        onChangeText={(val) => {
          if (val.includes(':+1:'))r {
            setMarkdownStyle({emoji: {fontSize: 30}});
          }
          setValue(val.replaceAll(':+1:', '👍'));
        }}
        style={styles.input}
        ref={ref}
        markdownStyle={markdownStyle}
        placeholder="Type here..."
        onSelectionChange={(e) => setSelection(e.nativeEvent.selection)}
        selection={selection}
      />

I updated the test steps ;)

@Skalakid Skalakid merged commit 4f9d790 into main Apr 25, 2024
5 checks passed
@Skalakid Skalakid deleted the @Skalakid/android-cursor-position-fix branch April 25, 2024 09:46
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