-
Notifications
You must be signed in to change notification settings - Fork 31
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
fix: Use picker for iris grid partition selector #2012
fix: Use picker for iris grid partition selector #2012
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spectrum picker doesn't have a small size, so we should remove the btn-sm
classes from the merge and keys buttons
label={column.name} | ||
table={partitionTables[index]} | ||
direction="bottom" | ||
shouldFlip={false} | ||
keyColumn={partitionTables[index].columns[index].name} | ||
placeholder={'Loading...' as string} | ||
labelColumn={partitionTables[index].columns[index].name} | ||
labelPosition="side" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would've probably grouped the label
and labelPosition
props together (since they're both labelling the component), but 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, I thought I should have labelColumn
and labelPosition
together, but what you have mentioned makes sense, since the labelColumn
is referring to something else. Should be good now
@dsmmcken Could you take another look at this whenever you have the time, so that I can merge? |
Thanks Don, merging now! |
Resolves #1970
Changes Implemented:
Testing Information:
Code used:
Sample visual of new Picker: