Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Support adding text labels to lines and shapes#6454

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
emilykl merged 86 commits intomasterfromadd-text-to-shapes
Mar 10, 2023

Conversation

@emilykl
Copy link
Contributor

@emilyklemilykl commentedJan 30, 2023
edited
Loading

Resolves#6430

Adds alabel property to Shapes allowing users to define text labels associated with any shape.label property has sub-propertytext to set the label text and other properties for setting position, styling, angle, etc. Label moves with shape during shape drag and plot drag.

shape.label API

  • label : Top-level property to be added toshape, holding all label options
    • text : The label’s text
    • font : The label’s font properties (same properties as forannotation)
      • color
      • family
      • size
    • textposition : The label’s position relative to the shape.
      • Possible values forlines:[ start | middle | end ]. Default:middle
      • Possible values forall other shapes:[ top | middle | bottom ] [ left | center | right ]. Default:middle center
    • textangle : Rotation angle of the label.
      • Possible values forlines: Number between -180 and 180, orauto (same angle as line). Default:auto
      • Possible values forall other shapes: Number between -180 and 180. Default: 0
    • xanchor : The x-component of the anchor point on the label used to determine position (in the pre-rotated reference frame). Possible values :[ auto | left | center | right ]. Default:auto, which has the following behavior —center if position iscenter, otherwise whatever places the label furthest towards theinside of the shape (e.g. if position istop right,xanchor defaults toright)
    • yanchor : The y-component of the anchor point on the label used to determine position (in the pre-rotated reference frame). Possible values :[ top | middle | bottom ]. Default:bottom for lines.middle for all other shapes if position ismiddle, otherwise whatever places the label furthest towards the inside of the shape (e.g. if position istop right,yanchor defaults totop)
    • padding : Offset of label relative toxanchor andyanchor (symmetrical on all sides)
      • x-padding is ignored whenxanchor iscenter
      • y-padding is ignored whenyanchor ismiddle
      • For lines with angleauto:padding is treated as a perpendicular offset from the line

Remaining loose ends (all resolved)

Heads up@archmoj@alexcjohnson -- I'll need help tying up these items.

  • Label position update during drag does not work wheneditable=True is set for individual shapes. (It does work wheneditable=True is set for entire plot.)
  • Addinglabel properties todraw_newshape?
  • Some Jasmine tests are timing out -- having a hard time debugging these
  • Feedback on image baselines
  • General feedback and suggestions

Partnership

Development of this feature is sponsored by Volkswagen's Center of Excellence for Battery Systems.

Copy link
Contributor

@archmojarchmoj left a comment

Choose a reason for hiding this comment

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

Very nice work.
Thanks@emilykl 🏆
Please find my first round comments & suggestions below.

emilykl reacted with heart emoji
'bottom left','bottom center','bottom right',
'top start','top end',
'middle start','middle end',
'bottom start','bottom end',
Copy link
Contributor

Choose a reason for hiding this comment

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

Are thesestart|end really necessary?
If so, let's describe them in the description please.

Copy link
ContributorAuthor

@emilyklemilyklJan 31, 2023
edited
Loading

Choose a reason for hiding this comment

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

Definitely open to suggestions here, but my thought was that corner-based positions liketop right,top left etc. don't really make sense for lines because imagine e.g. a downward sloping line --top right of the bounding box would place the label at a far-off point in space nowhere close to the line.

Even if the line started out sloping upward and sotop right made sense, it could be dragged by the user into a downward-sloping position.

I agree that mixingstart|end positions withtop|middle|bottom is a little convoluted though. One thought I had was that maybe lines should only support 4 specific label positions:top,bottom,start andend (or potentially,top,bottom,left andright). Not sure if that would be more or less confusing.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Maybe@alexcjohnson has thoughts here as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

My thinking about these attributes is thatposition refers to a single point on the shape, and[xanchor, yanchor] (pluspadding) refers to a single point on the text element, and you position the text so that those two points are in the same place.

So for lines, the onlyposition values that make sense arestart|middle|end. We can set smart defaults forxanchor based on those ({start: 'left', middle: 'center', end: 'right'} so that by default forposition: start|end the start or end of the text aligns with the start or end of the line, and forposition: middle the text is centered on the line. Thenyanchor: bottom puts the text on top of the line (ie the bottom of the text is on the line),yanchor: top puts the text under the line, andyanchor: 'middle' puts the text on top of the line. You can get other cool effects then like text preceding the line (position: start, xanchor: right, yanchor: middle) or following the line (position: end, xanchor: left, yanchor: middle)

For other shapes the first nine positions you have are good, and in those cases we can set smart defaults for bothxanchor andyanchor to match, ie[xanchorDefault, yanchorDefault] = position.split(' ') so (for rects anyway) by default the text is drawn just inside whatever corner or side you specify withposition. Default for all these shapes should bemiddle center though.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Yeah I think this makes sense@alexcjohnson , thanks!

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@alexcjohnson I've implemented these changes to theposition property for lines

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@alexcjohnson Oh, I realized I skimmed over this part of your comment before:

For other shapes the first nine positions you have are good, and in those cases we can set smart defaults for both xanchor and yanchor to match, ie [xanchorDefault, yanchorDefault] = position.split(' ') so (for rects anyway) by default the text is drawn just inside whatever corner or side you specify with position.

I've implemented the exact opposite -- by default the text is drawnoutside the corner, ifxanchor andyanchor are not specified. That feels more intuitive to me, and I think it generally looks better, so wanted to verify that that's okay. If it needs to be the other way for consistency with other parts of the API, I can change it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it depends what we consider the default or most common use case for labels on shapes. I feel like if I was using a shape to label a particular region of a graph - an important x or y range, typically - I'd likely want the label within that region, but off in a corner so it avoids most of the data in that region.

I guess that really only applies to rectangles, for circles or paths being outside the region would be better as otherwise the label would often overlap the edge of the shape, and there are other use cases even for rects where an outside label would make more sense. Perhaps though even then it would make sense forxanchor to be within the bounds and justyanchor to be outside? So for exampletextposition="top left" would default toxanchor="left", yanchor="bottom"?

Also: can we handle this logic at thedefaults stage instead of thedraw stage? If we do that, the choices we made show up infullLayout which can help some users understand and fix unwanted behavior. And at that point I don't see a use for"auto" in these attributes (which in turn means they can probably stop inheriting from annotations)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I guessxanchor on lines withtextposition='start'|'end' still needs'auto' because it depends whether the start is toward the left or right, and we can't know that unless we also know the axis range it's on. That doesn't apply toyanchor though, I still don't see a reason to let that be'auto'.

But a little more logic around this perhaps, just for lines: Iftextangle='auto' I would expect the text to be along the line by default, rather than extending off the end. Whereas for any othertextangle I'd expect the text to extend off the end, since if it tried to go along the line it would often overlap the line.

Copy link
ContributorAuthor

@emilyklemilyklMar 2, 2023
edited
Loading

Choose a reason for hiding this comment

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

@alexcjohnson I've updatedxanchor andyanchor defaults so they reflect the following behavior:

  • yanchor:
    • Default for lines isbottom
    • Default for all other shapes is such that the label is positionedinside the shape bounding box (e.g. iftextposition istop right,yanchor istop.
    • auto is not a supported value foryanchor
  • xanchor default is alwaysauto. This has the following behavior at render:
    • For lines:
      • Iftextangle isauto (parallel to line)xanchor is set so that text is inside the line bounding box
      • Iftextangle is anything else,xanchor is set in the opposite direction, so that text is outside the line bounding box
    • For other shapes:xanchor is set such that the label is positionedinside the shape bounding box (e.g. iftextposition istop right,xanchor isright

This mostly works pretty nicely. The only cases that are not ideal are circles and paths -- since the text is positioned inside the bounding box, it overlaps the circle/path in an odd way for any position besidesmiddle center.

We could potentially position the textoutside the bounding box by default for circles and paths... but not sure whether that's worth the additional complexity.

}),
position:{
valType:'enumerated',
values:[
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we need to haveautodflt andvalue in respect tosrc/components/shapes/defaults.js logic.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@archmoj What would be the use case for anauto value fortextposition?

Copy link
Contributor

Choose a reason for hiding this comment

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

@archmoj What would be the use case for anauto value fortextposition?

Hmm.. that's a good idea. But as far as I recall we don't have such an option fortextposition in other places. So perhaps we could keep this part as is.

@emilykl
Copy link
ContributorAuthor

Thanks so much for the first round review@archmoj !

@emilykl
Copy link
ContributorAuthor

@alexcjohnson@archmoj A couple dangling property naming questions:

  • @alexcjohnson Any thoughts on@archmoj 's comment above re: naming the top-level propertytitle rather thanlabel ? I likelabel better from a usability perspective but happy to change iftitle makes more sense in the larger scheme of things.
  • I thinkposition should be renamed totextposition since that property name seems more commonly used across the API (e.g. inplot_schema.json there are 5 properties namedposition compared to 13 namedtextposition) -- any objections?

emilykland others added23 commitsFebruary 20, 2023 18:06
description:[
'Sets the position of the label text relative to the shape.',
'Supported values for rectangles, circles and paths are',
'`top left`, `top center`, `top right`, `middle left`,',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that back quotes are used around attribute names not the values.
In all the new documentation, please place values inside* instead.
This line for example would become:

'*top left*, *top center*, *top right*, *middle left*,',

emilykl reacted with thumbs up emoji
@archmoj
Copy link
Contributor

We are almost there.
After updating the descriptions@emilykl please write the draft log for this PR (atdraftlogs/6454_add.md) & thank the sponsors possibly with a link to their website.

FYI - These files would be used during the release process to generate the changelog e.g.https://github.com/plotly/plotly.js/releases/tag/v2.17.0

Please seehttps://github.com/plotly/plotly.js/blob/master/draftlogs/README.md for more info.
Thank you!

@archmoj
Copy link
Contributor

@emilykl The PR initial description (#6454 (comment)) looks great! Is it up-to-date too?

emilykl reacted with heart emoji

@archmoj
Copy link
Contributor

---------
|\ |
|💃 |
| \|
---------

Excellent PR. Thanks@emilykl 🏆 🎖️ 🏅

emilykl and ndrezn reacted with heart emojindrezn reacted with rocket emoji

@emilykl
Copy link
ContributorAuthor

@archmoj Just updated the PR description!

archmoj reacted with hooray emojiarchmoj reacted with rocket emoji

@archmoj
Copy link
Contributor

@emilykl before merging this PR, would you mind pullorigin/master & mergemaster into this branch.
That's to make sure the new lint test we added in#6513 would pass here first.
Thank you!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@alexcjohnsonalexcjohnsonalexcjohnson left review comments

@LiamConnorsLiamConnorsLiamConnors left review comments

+1 more reviewer

@archmojarchmojarchmoj left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

featuresomething new

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support adding text labels to lines and shapes

6 participants

@emilykl@archmoj@JulianWgs@alexcjohnson@LiamConnors

[8]ページ先頭

©2009-2025 Movatter.jp