Skip to content
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

Deprecate extents prop perhaps? #179

Open
mhkeller opened this issue Mar 12, 2024 · 8 comments
Open

Deprecate extents prop perhaps? #179

mhkeller opened this issue Mar 12, 2024 · 8 comments
Labels
awaiting-something-else Needs some other thing to happen before it can be done discussion Talk about implementation of different chart types.

Comments

@mhkeller
Copy link
Owner

mhkeller commented Mar 12, 2024

Setting the domain via xDomain, yDomain, rDomain or zDomain is very similar to setting it via extents with the exception that domains set via extents skip measuring the extent for that dimension.

Instead of having this as a separate prop, dimensions could be skipped if a domain is passed into the _Domain props and it contains no null values.

In any event, this can wait until Svelte 5 is out since it would be a breaking change.

@mhkeller mhkeller added the discussion Talk about implementation of different chart types. label Mar 12, 2024
@mhkeller
Copy link
Owner Author

@techniq @rgieseke or @jtrim-ons are you using the extents prop at all? I think I added it for a very dynamic project I was working on and it seemed like a good at the time but I think the above solution provides the same benefits through _Domain props.

@techniq
Copy link
Contributor

techniq commented Mar 13, 2024

@mhkeller A quick search in LayerChart, I'm using it on some Horizontal / Vertical Bar examples (Grouped, Stacked, etc). I haven't check any other projects yet.

I'm out of town right now, but I can investigate using xDomain / yDomain instead in a few days. Thanks for the heads up.

@techniq
Copy link
Contributor

techniq commented Mar 14, 2024

@mhkeller Actually I found a minute and went ahead and replaced the LayerChart's extents={{ ... }} usage with xDomain / yDomain. Thanks for the heads up

@rgieseke
Copy link
Contributor

@mhkeller Thanks for the heads up, I didn't find any usage of the extents prop in my projects.

@jtrim-ons
Copy link
Contributor

Thanks @mhkeller . I don't think I have anything that uses extents. cc-ing @bothness...

@mhkeller mhkeller added the awaiting-something-else Needs some other thing to happen before it can be done label May 9, 2024
@mhkeller
Copy link
Owner Author

mhkeller commented May 9, 2024

To limit the number of breaking changes, I'm going to delay deprecating this until the Svelte 5 Runes rewrite.

@mhkeller
Copy link
Owner Author

mhkeller commented May 9, 2024

Thanks everyone for chiming in! Seems like removing this won't be that impactful. I'll see if I can put in a warning in the meantime maybe.

@techniq
Copy link
Contributor

techniq commented May 9, 2024

To limit the number of breaking changes, I'm going to delay deprecating this until the Svelte 5 Runes rewrite.

I'll be hot on your tail when you do :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-something-else Needs some other thing to happen before it can be done discussion Talk about implementation of different chart types.
Projects
None yet
Development

No branches or pull requests

4 participants