[No QA] Cleanup implementation of HTMLEngineProvider#6885
[No QA] Cleanup implementation of HTMLEngineProvider#6885stitesExpensify merged 14 commits intomainfrom
Conversation
roryabraham
changed the title
# Conflicts: # src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
MelvinBot
requested review from
stitesExpensify
and removed request for
a team
# Conflicts: # src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
|
Merged main again to keep up-to-date and make sure tests pass, but there were no conflicts. Small bump @stitesExpensify |
stitesExpensify
left a comment
stitesExpensify
left a comment
There was a problem hiding this comment.
LGTM, good cleanup IMO!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by @stitesExpensify in version: 1.1.31-2 🚀
|
|
🚀 Deployed to production by @AndrewGable in version: 1.1.32-0 🚀
|
| if (!HTMLEngineUtils.isInsideComment(props.tnode)) { | ||
| // This is not a comment from a chat, the AnchorForCommentsOnly uses a Pressable to create a context menu on right click. | ||
| // We don't have this behaviour in other links in NewDot | ||
| // TODO: We should use TextLink, but I'm leaving it as Text for now because TextLink breaks the alignment in Android. |
There was a problem hiding this comment.
Coming from this issue #80349 we should use TextLink instead of Text. I couldn't reproduce any Android issues when using it, so I think it's safe to make this change now unless someone has counterexamples.
There was a problem hiding this comment.
Really old PR, totally possible the alignment issue I described doesn't happen anymore. I'd say update it to use TextLink
4 participants
Details
This PR cleans up the
HTMLEngineProvidercomponent implementation to make it align with the one-component-per-file convention we have throughout the rest of our codebase. There was a seemingly arbitrary exception made inHTMLEngineProvider, and it made the component very large and harder to scan/reason about.Fixed Issues
$ n/a
Tests
QA Steps
Just regression tests.
Tested On
Screenshots
Web
Mobile Web
Desktop
iOS
Android