Skip to content

Add clarification on default selected/expanded IDs#221

Merged
dgreene1 merged 1 commit intodgreene1:masterfrom
dospunk:patch-1
May 5, 2025
Merged

Add clarification on default selected/expanded IDs#221
dgreene1 merged 1 commit intodgreene1:masterfrom
dospunk:patch-1

Conversation

@dospunk
Copy link
Contributor

@dospunk dospunk commented Apr 28, 2025

Adds clarification that defaultSelectedIds and defaultExpandedIds are only used if selectedIds and expandedIds are not provided (respectively).

@dgreene1
Copy link
Owner

Thanks @dospunk. @yhy-1 / @mellis481 if what our generous contributor wrote in this documentation PR is true, shouldn’t we use some kind of mutually exclusive typescript type so that both our consumers and our code clarifies that both props should not be sent?

I say this because there are many comments per year that express confusion about these props.

@mellis481
Copy link
Collaborator

Thanks @dospunk. @yhy-1 / @mellis481 if what our generous contributor wrote in this documentation PR is true, shouldn’t we use some kind of mutually exclusive typescript type so that both our consumers and our code clarifies that both props should not be sent?

I say this because there are many comments per year that express confusion about these props.

We inherited this repo after the separate props were implemented.

@dgreene1
Copy link
Owner

dgreene1 commented May 5, 2025

Thanks @dospunk. @yhy-1 / @mellis481 if what our generous contributor wrote in this documentation PR is true, shouldn’t we use some kind of mutually exclusive typescript type so that both our consumers and our code clarifies that both props should not be sent?
I say this because there are many comments per year that express confusion about these props.

We inherited this repo after the separate props were implemented.

Thanks for the history. But do you agree that a mutually exclusive interface would be helpful here? Or is this documentation update in this PR wrong?

@mellis481
Copy link
Collaborator

But do you agree that a mutually exclusive interface would be helpful here? Or is this documentation update in this PR wrong?

What is in the PR is correct. IIRC, other libraries (eg. Ant Design) do the same thing (different properties for default values and controlled values), but don't document it. I would not like to update the interface. I'm OK with merging this PR.

@dgreene1
Copy link
Owner

dgreene1 commented May 5, 2025

But do you agree that a mutually exclusive interface would be helpful here? Or is this documentation update in this PR wrong?

What is in the PR is correct. IIRC, other libraries (eg. Ant Design) do the same thing (different properties for default values and controlled values), but don't document it. I would not like to update the interface. I'm OK with merging this PR.

Sounds good then. Thanks @dospunk for giving us an easily mergeable and great addition to the library.

@dgreene1 dgreene1 merged commit 52983d7 into dgreene1:master May 5, 2025
1 check passed
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.

3 participants