Skip to content

fix frameanchor bottom #1061

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

Merged
merged 3 commits into from
Sep 30, 2022
Merged

fix frameanchor bottom #1061

merged 3 commits into from
Sep 30, 2022

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Sep 27, 2022

closes #1043

@Fil Fil requested review from enjalot and mbostock September 27, 2022 14:13
@Fil Fil force-pushed the fil/fix-frameanchor-bottom branch from fa9320b to 4e8dffa Compare September 27, 2022 15:00
),
Plot.text(["Count of penguins\ngrouped by species\n and colored by sex"], {
frameAnchor: "bottom-right",
dx: -3,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at the image, wouldn't it be nicer if dx was a positive number to avoid the overlap with the bar?
i guess that's a bit fiddly, could we right align the text?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, it doesn't look like this on my computer.

In fact, it's worse than this: when I see the image on the vite server, it's perfect. When I look at it in the Finder, the right alignment is broken. And on github everything is broken… Why is that???

vite server

Finder view

Github View

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when i download the svg and remove .plot tspan { white-space: pre; }
it fixes the text anchor end issue.
i'm not seeing line-height or dx on the text marks in the svg though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line-height becomes the various tspan elements' y attributes; and dx is factored in the x attribute of the text element.

Apparently the faulty views are caused by the pretty formatting of the svg with beautify.html in

const actual = beautify.html(root.outerHTML, {indent_size: 2});
— combined with the white-space: pre; of the text elements, it creates these fake spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This side-effect of testing will be fixed after #1062.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's looking good. Thanks @enjalot for catching this!
image

Fil added a commit that referenced this pull request Sep 28, 2022
…tain tspans.

These result in unreliable positioning.
See #1061 (comment)
Fil added a commit that referenced this pull request Sep 29, 2022
…tain tspans. (#1062)

These result in unreliable positioning.
See #1061 (comment)
@Fil Fil force-pushed the fil/fix-frameanchor-bottom branch from 4e8dffa to 4fcf7dd Compare September 29, 2022 07:32
@Fil Fil merged commit f6d13d1 into main Sep 30, 2022
@Fil Fil deleted the fil/fix-frameanchor-bottom branch September 30, 2022 14:27
@mbostock
Copy link
Member

This PR broke the alignment of labels on scatterplots.

Here is a frameAnchor of top:

Screen Shot 2022-11-23 at 1 48 30 PM

The frameAnchor of bottom is supposed to be symmetric, with the baseline of the text touching the anchor:

Screen Shot 2022-11-23 at 1 48 54 PM

This PR shifts the bottom anchor by 1.29 lines, resulting in dots that are significantly farther away than intended by the anchor:

Screen Shot 2022-11-23 at 1 49 40 PM

Perhaps the problem in #1043 is specific to multiline text. I will investigate but we should revert this and apply a different fix.

frontend-provider pushed a commit to frontend-provider/plot that referenced this pull request Sep 20, 2023
…tain tspans. (#1062)

These result in unreliable positioning.
See observablehq/plot#1061 (comment)
backend-devloper pushed a commit to backend-devloper/plot that referenced this pull request Nov 24, 2023
…tain tspans. (#1062)

These result in unreliable positioning.
See observablehq/plot#1061 (comment)
tigrevol8888 added a commit to tigrevol8888/plot that referenced this pull request Jul 5, 2024
…tain tspans. (#1062)

These result in unreliable positioning.
See observablehq/plot#1061 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

frameAnchor: bottom is not properly aligned
3 participants