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.

Sign up for GitHub

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

Jump to bottom

feat(annotation): consider using <foreignObject> for text layout #1173

Open
williaster opened this issue Apr 16, 2021 · 3 comments
Open

feat(annotation): consider using <foreignObject> for text layout #1173

williaster opened this issue Apr 16, 2021 · 3 comments
Labels
🚀enhancement 👋 help wanted @visx/annotation

Comments

@williaster
Copy link
Collaborator

From #1127 (comment)

@williaster reading the discussion/problem statements here, I wonder if using with HTML elements would help/simplify in some way? Could that offer automatic text reflow?

@felixfbecker that's an interesting idea, we haven't really used that in visx at all. For @visx/legend and @visx/tooltip we opted to make them HTML based to get layout for free, and we have @visx/text for cases where you really probably should use SVG s.
Annotations were initially implemented with SVG to get something out and usable, but we discussed having an HTML based variant as well at some point. could be interesting to consider instead, though.
valtism added a commit to valtism/visx that referenced this issue Nov 26, 2021
@valtism
Annotation label <foreignobject> render
78295bf 
Add option to pass in an element to render as a label as a child.
Uses <foreignObject> to allow user to pass in an HTML element.

Related to this discussion: airbnb#1173

This has the benefit of letting the element handle text reflow, with
the drawback being that the user need to manage min and max width of
the container element they render.

This approach seems much better imo, favouring composability over
configuration which is much more in line with the philosophy of d3.
@valtism valtism mentioned this issue Nov 26, 2021
Merged
@richsilv
Copy link

One consideration to bear in mind is that most non-browser SVG renderers will not render HTML <foreignObject>s. This means that if you produce charts with legends or axis labels which use <foreignObject>s in the browser and then try to copy/paste them into a document, those elements might simply not appear. Of course, you can always use screenshots instead, but particularly if there is any use case which involves tweaking of the vector graphics before onward use in presentations (which I think is a not-uncommon business use case), migrating to <foreignObject> could make visx unusable.

@williaster
Copy link
Collaborator Author

Interesting point @richsilv, perhaps it's worth noting that in the docs for the new label type. I think both svg + html would be supported so users could then make their own decision based on different use cases.

williaster pushed a commit that referenced this issue Jan 21, 2022
@valtism
Annotation label <foreignobject> render (#1383)
ae47109 
* Annotation label <foreignobject> render

Add option to pass in an element to render as a label as a child.
Uses <foreignObject> to allow user to pass in an HTML element.

Related to this discussion: #1173

This has the benefit of letting the element handle text reflow, with
the drawback being that the user need to manage min and max width of
the container element they render.

This approach seems much better imo, favouring composability over
configuration which is much more in line with the philosophy of d3.

* Fix child render for Safari

Add witdth and height to <foreignObject>. Worked for Chrome but was not
displaying anything for Safari.

* Fix test cases

Not sure how idomatic all the `dive()`s are, but I've heard complaints
about them so I'm guess it's the way to go.

* Remove console log

* Address PR comments

Rename ForeignObjectLabel -> HtmlLabel

Split out HtmlLabel and LabelAnchorLine to separate components.

Export HtmlLabel as a separate component as it uses a small subset of the shared props.
@dreamyguy
Copy link

I used <foreignObject> as a way to get text to respect its parent boundaries. Everything looked great until we started looking at compatibility.

Safari does not support it – at all – while most browsers are quirky. Very poor documentation on how to use this right. This is probably going to be suddenly dumped by all browsers.

That besides all that @richsilv already mentioned. Non-browser support is null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🚀enhancement 👋 help wanted @visx/annotation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dreamyguy @richsilv @williaster