Allow truncating SVG in AppNodeText #945
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for monarch-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #945 +/- ##
==========================================
+ Coverage 70.97% 71.39% +0.42%
==========================================
Files 91 92 +1
Lines 3128 3188 +60
==========================================
+ Hits 2220 2276 +56
- Misses 908 912 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ptgolden
force-pushed
the
truncating-app-node-text
branch
from
877ffa1 to
b7cc485
Compare
Additionally, use AppNodeText for SVG labels
ptgolden
force-pushed
the
truncating-app-node-text
branch
from
b7cc485 to
a7982af
Compare
|
I am going to add several test cases before continuing |
This commit does two main things. First, it almost completely dials back the difference between rendering HTML and SVG. Instead of recreating HTML elements (<b>, <i>, <sup>) with styled <tspan> elements in SVG, it just includes those elements as HTML in a <foreignObject> tag. Part of the reason I did this is that there were strange cross-browser differences in how superscripted text was being truncated. For some reason, the code for detecting character positions (see the code that was removed with `getStartPositionOfChar` and `getCharNumAtPosition`) stopped working when `dy` was set on a tspan in some browsers. There was also the issue that the SVG equivalents to the HTML elements were always sliiightly different. --- Second, and following from that, I was able to get rid of a lot of the complexities that went with treating rendering SVG and HTML differently. The new logic is much more simple: the text of the label is retrieved via `innerHtml`, then a whitelist of tags is escaped (i.e. replacing `<` with `<` and `>` with `>`). The only SVG specific logic (besides wrapping the container in a <foreignObject>) is replacing <sup> tags with <span> tags styled as superscript. (Again, because of cross-browser rendering issues, which are commented in the code).
ptgolden
force-pushed
the
truncating-app-node-text
branch
from
4fe5bd2 to
9813cf2
Compare
amc-corey-cox
approved these changes
May 27, 2025
|
Waiting to merge until you make that last small fix. |
|
@ptgolden this is going to have conflicts now with all the updates @varun-divya has done. If it still needs to happen, please work with her to get the conflicts resolved. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Labels in the phenogrid were previously rendered as
<text>SVG elements, rather than<AppNodeText>Vue elements which whitelist certain markup. This replaces thosetextelements withAppNodeTextelements.An issue, though, is that labels were previously truncated based on their bare text length. So the string
'label<b>with bolded</b>'might be truncated to'label<b'or'label<b>with bo'rather than'label with bo'(where "with bo" would be bolded).This PR fixes that bug by truncating text according to its length as it appears in rendered SVG.
Fixes the bug mentioned in #943 (comment).