Skip to content

feat: Add unifiedhovertitle to format unified hover title #7439

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Jun 12, 2025

Resolves #7427.
TODO:

  • finalized attribute name
  • add jasmine tests

@archmoj archmoj added feature something new cs customer success labels Jun 12, 2025
@archmoj archmoj changed the title add: Add unifiedhovertemplate to format unified hover title feat: Add unifiedhovertemplate to format unified hover title Jul 8, 2025
@gvwilson
Copy link
Contributor

gvwilson commented Jul 10, 2025

@LiamConnors @emilykl please give @archmoj feedback on the naming here.

@gvwilson gvwilson requested review from LiamConnors and emilykl July 10, 2025 16:09
@emilykl
Copy link
Contributor

emilykl commented Jul 15, 2025

@archmoj @gvwilson Maybe unifiedhovertitle instead?

@archmoj archmoj changed the title feat: Add unifiedhovertemplate to format unified hover title feat: Add unifiedhovertitle to format unified hover title Jul 15, 2025
@archmoj
Copy link
Contributor Author

archmoj commented Jul 15, 2025

@archmoj @gvwilson Maybe unifiedhovertitle instead?

Thanks @emilykl I rename it to unifiedhovertitle.
Now this raises the question that if we should make it a container to allow adding more options in future?
For example:

{
  "unifiedhovertitle": {
    "text": "", 
    "font": {}
  }
}

@emilykl
Copy link
Contributor

emilykl commented Jul 15, 2025

@archmoj Do we support those formatting options for other parts of the hovertext? Probably be consistent with whatever we do elsewhere for the hovertext (and, should we want to add formatting options, add them everywhere in the hovertext at the same time).

@emilykl
Copy link
Contributor

emilykl commented Jul 15, 2025

@archmoj If you're concerned about the word title in the attribute name implying that the attribute should be passed as a container -- perhaps we could call it unifiedhoverheader ?

@gvwilson
Copy link
Contributor

gvwilson commented Jul 15, 2025

Thanks for the discussion in the team meeting - just to check my understanding:

  1. We're considering introducing a container to be consistent with other places where we have titles.
  2. This would be a breaking change to our API.

Are both correct?

@emilykl
Copy link
Contributor

emilykl commented Jul 15, 2025

  • We're considering introducing a container to be consistent with other places where we have titles.

Yes

  • This would be a breaking change to our API.

The unifiedhovertitle attribute does not exist yet — it's added by this PR — so if we were to add it as a container in this PR it would not be a breaking change.

If we introduce it as a simple text attribute now, and then later change it to be a container, that would be a breaking change.

@gvwilson
Copy link
Contributor

in that case I vote for making a container - @archmoj is this your preferred solution?

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

Successfully merging this pull request may close these issues.

Allow customization of the first line of the hovertext in x/y unified hovermode
3 participants