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

WIP: SelectableTileGroup #439

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

Conversation

ispyinternet
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Nov 30, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/carbon-svelte/carbon-components-svelte/plux0963q
✅ Preview: https://carbon-components-svelte-git-master.carbon-svelte.vercel.app

src/Tile/TileGroup.svelte Outdated Show resolved Hide resolved
src/Tile/RadioTile.svelte Outdated Show resolved Hide resolved
src/Tile/RadioTile.svelte Outdated Show resolved Hide resolved
src/DataTableSkeleton/DataTableSkeleton.svelte Outdated Show resolved Hide resolved
src/Tile/RadioTile.svelte Outdated Show resolved Hide resolved
@metonym
Copy link
Collaborator

metonym commented Nov 30, 2020

In addition to passing the CI/deploy preview, could you remove the committed package-lock.json files?

@ispyinternet
Copy link
Contributor Author

In addition to passing the CI/deploy preview, could you remove the committed package-lock.json files?

Should I update .gitignore?

I also inadvertantly added palimpsest, should I add this to gitignore?

@ispyinternet
Copy link
Contributor Author

ispyinternet commented Nov 30, 2020

BTW, you said in the issue comments to use selectedItem selectedItems which also is breaking? Wasn't sure if you realised?

I'm leaning towards keeping them separate. Preserve the TileGroup component but add two new ones dedicated to the SelectableTile, RadioTile respectively:

SelectableTileGroup – bind:selectedValues
RadioTileGroup – bind:selectedValue

@metonym
Copy link
Collaborator

metonym commented Nov 30, 2020

How about creating separate PRs that address the issues individually?

@metonym
Copy link
Collaborator

metonym commented Nov 30, 2020

BTW, you said in the issue comments to use selectedItem selectedItems which also is breaking? Wasn't sure if you realised?

selectedValue and selectedValues would be new props if SelectableTileGroup and RadioTileGroup components are introduced.

@ispyinternet
Copy link
Contributor Author

ispyinternet commented Dec 10, 2020

@metonym I have to admit I'm stumped on the last example. I can't get an atomic update of the store. Basically, I'm reasoning that as the control updates the each loop, when the first item updates, the store updates, which passes an update back down to the children - it will only reset one item at a time. I attempted to use #key but that doesn't fix it:

https://carbon-components-svelte-plux0963q.vercel.app/framed/SelectableTile/SelectableTileReactive

@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

8 similar comments
@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ibm-ci-bot
Copy link

@ispyinternet: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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