Skip to content

Add possibility to include a data attribute to text element. #1841

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

Closed
wants to merge 1 commit into from

Conversation

lotusjohn
Copy link

@lotusjohn lotusjohn commented Aug 29, 2023

I want to be able to scale text with css to offset text scaling together with the svg.
Ex, svg width=1000 and fontSize=16, if this svg then has a width: 100% with css on it, the text will be super small if the svg container scales down on for ex mobile devices.

With this we can now choose to add a data attribute to the text node which can be selected in css. This enables us to target different text nodes in the svg to apply different styling on different nodes.

A really nice example is using container query units to offset the scaling text with css only. Then we can just do something like this with the cqi unit.

.plotcontainer text[data-name="my-regular-text"] {
    font-size: calc(2rem - 1.85cqi);
}

.plotcontainer text[data-name="my-large-text"] {
    font-size: calc(8rem - 2cqi);
}

@mbostock
Copy link
Member

I would prefer to solve this at the base Mark level rather than being specific to the Text mark.

Could you use a class name #1002 or the existing ariaLabel or ariaDescription options to target the elements instead? Seems like that would already work.

Regarding data attributes, would you want these to be channels (varying on each instance of a mark), or just constant? It sounds like a constant would be sufficient here, and simpler. If a channel, we would have to declare it and initialize it in the constructor.

@lotusjohn
Copy link
Author

You're right, className is probably a better way to go for this.

Regarding channels or not, im not super clear on this at the moment, so far i've only had use cases where it would not have been necessary.

@lotusjohn lotusjohn closed this Sep 6, 2023
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.

2 participants