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

Allow setting placeholder for st.multiselect #6901

Merged
merged 10 commits into from
Jul 10, 2023
2 changes: 1 addition & 1 deletion e2e/scripts/st_multiselect.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def set_multiselect_9_to_have_bad_state():

options = ("male", "female")

i1 = st.multiselect("multiselect 1", options)
i1 = st.multiselect("multiselect 1", options, placeholder="Please select")
st.text(f"value 1: {i1}")

i2 = st.multiselect("multiselect 2", options, format_func=lambda x: x.capitalize())
Expand Down
2 changes: 1 addition & 1 deletion e2e/specs/st_multiselect.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe("st.multiselect", () => {
it("should show the correct placeholder", () => {
cy.get(".stMultiSelect")
.first()
.should("have.text", "multiselect 1Choose an optionopen");
.should("have.text", "multiselect 1Please selectopen");
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const getProps = (elementProps: Partial<MultiSelectProto> = {}): Props => ({
label: "Label",
default: [0],
options: ["a", "b", "c"],
placeholder: "Please select",
...elementProps,
}),
width: 0,
Expand Down Expand Up @@ -114,9 +115,7 @@ describe("Multiselect widget", () => {
it("renders when it's not empty", () => {
const props = getProps()
const wrapper = mount(<Multiselect {...props} />)
expect(wrapper.find(UISelect).prop("placeholder")).toBe(
"Choose an option"
)
expect(wrapper.find(UISelect).prop("placeholder")).toBe("Please select")
})

it("renders with empty options", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class Multiselect extends React.PureComponent<Props, State> {
const { options } = element
const disabled = options.length === 0 ? true : this.props.disabled
const placeholder =
options.length === 0 ? "No options to select." : "Choose an option"
options.length === 0 ? "No options to select." : element.placeholder
const selectOptions: MultiselectOption[] = options.map(
(option: string, idx: number) => {
return {
Expand Down
18 changes: 12 additions & 6 deletions lib/streamlit/elements/multiselect.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,10 @@ def multiselect(
args: Optional[WidgetArgs] = None,
kwargs: Optional[WidgetKwargs] = None,
*, # keyword-only arguments:
max_selections: Optional[int] = None,
placeholder: str = "Choose an option",
disabled: bool = False,
label_visibility: LabelVisibility = "visible",
max_selections: Optional[int] = None,
) -> List[T]:
r"""Display a multiselect widget.
The multiselect widget starts as empty.
Expand Down Expand Up @@ -213,6 +214,11 @@ def multiselect(
An optional tuple of args to pass to the callback.
kwargs : dict
An optional dict of kwargs to pass to the callback.
max_selections : int
The max selections that can be selected at a time.
This argument can only be supplied by keyword.
placeholder : str
An optional string to display when option is not selected.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe "A string to display when no options are selected. Defaults to 'Choose an option'"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d14d375
Thank you for the suggestion. It would be better to specify the default value.

disabled : bool
An optional boolean, which disables the multiselect widget if set
to True. The default is False. This argument can only be supplied
Expand All @@ -222,9 +228,6 @@ def multiselect(
is still empty space for it above the widget (equivalent to label="").
If "collapsed", both the label and the space are removed. Default is
"visible". This argument can only be supplied by keyword.
max_selections : int
The max selections that can be selected at a time.
This argument can only be supplied by keyword.

Returns
-------
Expand Down Expand Up @@ -258,10 +261,11 @@ def multiselect(
on_change=on_change,
args=args,
kwargs=kwargs,
max_selections=max_selections,
placeholder=placeholder,
disabled=disabled,
label_visibility=label_visibility,
ctx=ctx,
max_selections=max_selections,
)

def _multiselect(
Expand All @@ -276,10 +280,11 @@ def _multiselect(
args: Optional[WidgetArgs] = None,
kwargs: Optional[WidgetKwargs] = None,
*, # keyword-only arguments:
max_selections: Optional[int] = None,
placeholder: str = "Choose an option",
disabled: bool = False,
label_visibility: LabelVisibility = "visible",
ctx: Optional[ScriptRunContext] = None,
max_selections: Optional[int] = None,
) -> List[T]:
key = to_key(key)
check_callback_rules(self.dg, on_change)
Expand All @@ -296,6 +301,7 @@ def _multiselect(
multiselect_proto.options[:] = [str(format_func(option)) for option in opt]
multiselect_proto.form_id = current_form_id(self.dg)
multiselect_proto.max_selections = max_selections or 0
multiselect_proto.placeholder = placeholder
if help is not None:
multiselect_proto.help = dedent(help)

Expand Down
10 changes: 10 additions & 0 deletions lib/tests/streamlit/elements/multiselect_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ def test_defaults(self, defaults, expected):
self.assertEqual(c.label, "the label")
self.assertListEqual(c.default[:], expected)
self.assertEqual(c.options, ["Coffee", "Tea", "Water"])
self.assertEqual(c.placeholder, "Choose an option")

@parameterized.expand(
[
Expand Down Expand Up @@ -365,3 +366,12 @@ def test_get_over_max_options_message(
_get_over_max_options_message(current_selections, max_selections),
expected_msg,
)

def test_placeholder(self):
"""Test that it can be called with placeholder params."""
st.multiselect(
"the label", ["Coffee", "Tea", "Water"], placeholder="Select your beverage"
)

c = self.get_delta_from_queue().new_element.multiselect
self.assertEqual(c.placeholder, "Select your beverage")
7 changes: 4 additions & 3 deletions proto/streamlit/proto/MultiSelect.proto
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ message MultiSelect {
string form_id = 6;
repeated int32 value = 7;
bool set_value = 8;
bool disabled = 9;
LabelVisibilityMessage label_visibility = 10;
int32 max_selections = 11;
int32 max_selections = 9;
string placeholder = 10;
bool disabled = 11;
LabelVisibilityMessage label_visibility = 12;
Copy link
Collaborator

@vdonato vdonato Jul 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here as in #6913 (comment) we'll need to keep tag numbers the same for existing fields and add placeholder as tag 12.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d14d375
I understood that it is not necessary to match the order of the arguments. Thanks for telling me.

}