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

Text highlight does not get updated as text changes #7569

Closed
rozele opened this issue Apr 10, 2021 · 3 comments · Fixed by #8408
Closed

Text highlight does not get updated as text changes #7569

rozele opened this issue Apr 10, 2021 · 3 comments · Fixed by #8408

Comments

@rozele
Copy link
Collaborator

rozele commented Apr 10, 2021

Steps To Reproduce

Provide a detailed list of steps that reproduce the issue.

  1. Clone RNTester example: https://github.com/rozele/react-native-windows/tree/pressableTextRepro
  2. Open Text component example
  3. Scroll to "Pressable edge cases" example
  4. On the line that says "Disabling pressables is okay too.", click "Click to disable".
  5. Observe that the text highlighting is not removed for the "Click here." text and the highlighting is not updated for the "Click to enable." text.

Expected Results

Text highlighting updates with changes to nested text.

Snack, code example, screenshot, or link to a repository:

RNTester example: https://github.com/rozele/react-native-windows/tree/pressableTextRepro

playground.2021-04-10.10-57-47.mp4
@rozele rozele added the bug label Apr 10, 2021
@ghost ghost added Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) Partner: Facebook labels Apr 10, 2021
@rozele
Copy link
Collaborator Author

rozele commented Apr 10, 2021

FYI @koljaka - I believe you have a fix for this.

@chrisglein chrisglein added Area: Text and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Apr 12, 2021
@chrisglein chrisglein added this to the 0.65 milestone Apr 12, 2021
@rectified95
Copy link
Contributor

@rozele @koljaka Let me know if you have a fix already, so I can assign the issue to you. If you don't, I'll fix it.

@koljaka
Copy link
Contributor

koljaka commented Apr 12, 2021

@rectified95 It appears that my fix does not work in all cases, so it'd be better if you fix it. Thanks!

@rectified95 rectified95 self-assigned this Apr 12, 2021
@rectified95 rectified95 modified the milestones: 0.65, 0.66 Apr 23, 2021
@rozele rozele assigned rozele and unassigned rectified95 Aug 11, 2021
rozele added a commit to rozele/react-native-windows that referenced this issue Aug 11, 2021
There were two issues with the Text backgroundColor prop:
1. Changes to the backgroundColor prop on virtual text nodes did not
update their corresponding TextHighlighters
2. Changes to the underlying text did not update the TextHighlighter
ranges

This change forces a refresh of all TextHighlighters any time:
1. A raw text node changes text value
2. A virtual text node is added or removed from the text tree
3. A virtual text or root text node updates its backgroundColor prop

Fixes microsoft#7569
Fixes microsoft#8407
NickGerleman pushed a commit that referenced this issue Aug 19, 2021
* Fixes issues with Text backgroundColor

There were two issues with the Text backgroundColor prop:
1. Changes to the backgroundColor prop on virtual text nodes did not
update their corresponding TextHighlighters
2. Changes to the underlying text did not update the TextHighlighter
ranges

This change forces a refresh of all TextHighlighters any time:
1. A raw text node changes text value
2. A virtual text node is added or removed from the text tree
3. A virtual text or root text node updates its backgroundColor prop

Fixes #7569
Fixes #8407

* Change files

* yarn format

* Ensure inherited foreground color is applied to highlighters

* Fixes issue where single run text optimization backgroundColor failed

* Fixes a few bugs where text highlighters did not update properly

* Adds modified snapshots based on new highlights approach

* Adds RNTester example with dynamic backgroundColor props

* Adds helper for shadow node type conditionals

* Optimizes text tree updates to only DFS for highlight updates if needed

* Minor bug fix to set flag indicating TextHighlight may be required when setting foreground
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants