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!: shim an implementation of getSubStringLength (#6670) (CP: 23.5) #6673

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

DiegoCardoso
Copy link
Contributor

Manual CP from #6670 to 23.5

…4) (#6670)

* Shim an implementation of `getSubStringLength` (#6663)

* Shim an implementation of getSubStringLength

Estimate the rendered length of a substring of text in an SVG element. Supports
wrapping/truncation of long labels in charts.

* Improve shimmed measurement routines

Improve shims of getBBox and getSubStringLength:

The prior implementations were relying on specific DOM structures that
Highcharts used to create: if the measured element had children, all text
was expected to be within those children, and none in the top-level element.
Also, it did not handle arbitrary element nesting. The current version of
Highcharts does not always follow these rules, and there were cases of
structures like:

<text>Some text here<tspan>and some in a nested element</tspan></text>

In that example, 'Some text here' would be omitted from measurement.

Replace that strategy with one that processes each text node separately,
providing the context of the containing element, and recursing into all
nested elements.

Fix some bugs around new line management in getBBox that became apparent
after that change.

The previous method of computing string width was a rough approximation based
on average character width. This method produced suboptimal results in many
situtations, generating extra space or overlapping text.

Instead, use the string-pixel-width library to provide a better estimate. This
library has per-character widths for a number of font families and variants, and
results in widths much closer to the actual rendered style. As Lucida is used
heavily in at least the test charts, and it is not natively supported by the
library, add a custom mapping file including metrics for it.

Also, improve detection of font family and font size, walking up the element
parent chain (even potentially outside of the measured element) to find settings
of these attributes.

* Review cleanup

* Test getSubStringLength

Add test for getSubStringLength to ensure proper handling of sub-strings
crossing text nodes (and other cases).

To support this, change the way methods are added to jsdom SVG elements:
rather than adding them only for elements instantiated via createElementNS,
add them to the SVGElement prototype. This makes them available for elements
created by any method of instantiation (including by setting innerHTML, as used
in the test).

Use rewire to access private elements of the exporter during tests.

* spotless

* chore: pin npm dependencies

---------

Co-authored-by: Aron Nopanen <[email protected]>
Co-authored-by: Diego Cardoso <[email protected]>
@vursen vursen changed the title fix!: shim an implementation of getSubStringLength (#6663) (CP: 24.4) (#6670) fix!: shim an implementation of getSubStringLength (#6670) (CP: 23.5) Sep 27, 2024
Copy link

sonarcloud bot commented Sep 27, 2024

@DiegoCardoso DiegoCardoso merged commit cada66c into 23.5 Sep 27, 2024
5 checks passed
@DiegoCardoso DiegoCardoso deleted the cp/23.5/charts/shim-getsubstringlength branch September 27, 2024 08:15
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.5.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants