feat(toggle-button-group): removed columns and changed ul layout #2199
Conversation
🦋 Changeset detectedLatest commit: 23c24e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -4,7 +4,7 @@ $ const titles = ["Option 1", "Option 2", "Option 3"]; | |||
| <ebay-button onClick("clearSelection")> | |||
There was a problem hiding this comment.
I just noticed this. What's this for? Is it needed? Looks odd.
There was a problem hiding this comment.
This was there earlier
Will leave it in. This is good for devs to see
| @@ -0,0 +1,26 @@ | |||
| <ebay-toggle-button-group columnsSM=3 columnsXS=2 columnsMD=6 columnsXL=8 ...input> | |||
There was a problem hiding this comment.
Do we need columns and controlled? Aren't they the same thing?
Maybe we can combine them into a single one, titled "Exception Cases" or "Preferred Columns?"
There was a problem hiding this comment.
Controlled is an important code example for devs who want to hold state in the parent instead of letting our component do it, but it may not be necessary to have in storybook
There was a problem hiding this comment.
Will leave it. There's no other way for devs to know how to use columns
| @@ -0,0 +1,26 @@ | |||
| <ebay-toggle-button-group columnsSM=3 columnsXS=2 columnsMD=6 columnsXL=8 ...input> | |||
There was a problem hiding this comment.
The default state of this example that has columns defined (see markup) doesn't match the options underneath that are all null. That might be confusing to devs.
There was a problem hiding this comment.
Yes
This is fine. This is because we are setting it explicitly on the code
See show code button
We need an example with it set by default.
Description
columns.References
#2194