Skip to content

Align hover label text content with *hoverlabel.align* #3753

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 4 commits into from
Apr 9, 2019

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Apr 9, 2019

resolves #3726 by adding a new attribute under the hoverlabel trace and layout containers.

See demo: https://codepen.io/etpinard/pen/MRJvLr?editors=1010

I picked align as its behavior reminded me of align in the annotations attributes:

align: {
valType: 'enumerated',
values: ['left', 'center', 'right'],
dflt: 'center',
role: 'style',
editType: 'arraydraw',
description: [
'Sets the horizontal alignment of the `text` within the box.',
'Has an effect only if `text` spans more two or more lines',
'(i.e. `text` contains one or more <br> HTML tags) or if an',
'explicit width is set to override the text width.'
].join(' ')
},

Let me know if you can think of a better name.

cc @plotly/plotly_js @nicolaskruchten

etpinard added 4 commits April 9, 2019 10:40
... using their layout hoverlabel counterparts
- with values 'left', 'right' and 'auto' (the dflt and current behavior)
@etpinard etpinard added this to the v1.47.0 milestone Apr 9, 2019
@nicolaskruchten
Copy link
Contributor

The default is center? I would have thought the default would be “auto” to describe the current behaviour

@nicolaskruchten
Copy link
Contributor

Also re the description: this doesn’t just apply to “texts right? It applies to the entire contents of the hoverlabel, even if it’s built with hovertemplate etc?

@antoinerg
Copy link
Contributor

antoinerg commented Apr 9, 2019

Also re the description: this doesn’t just apply to “texts right? It applies to the entire contents of the hoverlabel, even if it’s built with hovertemplate etc?

hovertemplate was implemented so that it reuses all the existing logic. So yes the hoverlabel.align does apply to the entire content of the hoverlabel even if it's built with hovertemplate!

I think we could replace `text` by text in the description.

The default is center? I would have thought the default would be “auto” to describe the current behaviour

The default is actually auto. However, center is not yet implemented in this PR. @nicolaskruchten is this a requirement?

@etpinard
Copy link
Contributor Author

etpinard commented Apr 9, 2019

The default is center? I would have thought the default would be “auto” to describe the current behaviou

The default is 'auto'.

@etpinard
Copy link
Contributor Author

etpinard commented Apr 9, 2019

Wow. Sorry I made things confusing. The block I pasted in the PR description is in annotations/attributes.js not the new hoverlabel.align attribute declaration which is:

        align: {
            valType: 'enumerated',
            values: ['left', 'right', 'auto'],
            dflt: 'auto',
            role: 'style',
            editType: 'none',
            description: [
                'Sets the horizontal alignment of the text content within hover label box.',
                'Has an effect only if the hover label text spans more two or more lines'
            ].join(' ')
        },

as found in fx/layout_attributes.js.

@antoinerg
Copy link
Contributor

@etpinard about your #3726 (comment) . Since there are two different things we may want to align, wouldn't it be safer to call this new attribute textalign instead of align. I could see align being used to align with respect to the data points as you pointed out whereas in the current PR we deal with text alignment.

@etpinard
Copy link
Contributor Author

etpinard commented Apr 9, 2019

Since there are two different things we may want to align, wouldn't it be safer to call this new attribute textalign instead of align

Fair. But you could say the same thing about annotations.

@nicolaskruchten
Copy link
Contributor

center is not important to me BTW :)

@etpinard
Copy link
Contributor Author

etpinard commented Apr 9, 2019

center is not important to me BTW :)

🎉 that's what I thought.

@nicolaskruchten
Copy link
Contributor

✨ from me :)

@antoinerg
Copy link
Contributor

💃 once the tests pass!

@etpinard
Copy link
Contributor Author

etpinard commented Apr 9, 2019

Going with hoverlabel.align for consistency with annotations.

@nicolaromano
Copy link

Would it be possible to allow center-aligning text as well?

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

Successfully merging this pull request may close these issues.

Hoverlabel contents alignment
4 participants