Skip to content
Merged
2 changes: 1 addition & 1 deletion components/dash-core-components/src/fragments/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ const Dropdown = (props: DropdownProps) => {

setDisplayOptions(sortedOptions);
}
}, [filteredOptions, isOpen]);
}, [filteredOptions, isOpen, sanitizedValues]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding this to the dependency array does appear to fix the bug, but I think we can fix it at a deeper level and avoid the side effects that are seen in the failing tests.

I believe the root bug is actually in optionRendering.tsx where we render the component via an ExternalWrapper.

Here, the componentPath (required for the dash renderer) is constructed using the option's array index:

componentPath={[...ctx.componentPath, index, i]}

I believe this is the problem. When we select an option, its index changes (it moves to the top of the list) but we don't have a way to inform the renderer. So, internally, it becomes confused about which component is at what index. It's confused until you re-open the dropdown or force a re-render of everything, which is what your original fix did.

We can instead construct a componentPath using the option's value (which should be unique across options):

componentPath={[...ctx.componentPath, String(value), i]}

This will allow the renderer to keep a stable path to use internally and the index won't matter.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! That works, and this solution has better performance.

I did have to change the test I added because the options do not change position while the dropdown is open, but I think that's a better UX


// Focus first selected item or search input when dropdown opens
useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest
from dash import Dash, Input, Output
from dash.dcc import Dropdown
from dash.html import Div, Label, P
from dash.html import Div, Label, P, Span
from selenium.common.exceptions import TimeoutException
from selenium.webdriver.common.keys import Keys
from selenium.webdriver.common.action_chains import ActionChains
Expand Down Expand Up @@ -434,3 +434,40 @@ def is_visible(el):
)

return all([is_visible(el) for el in elements])


def test_a11y009_dropdown_component_labels_render_correctly(dash_duo):
app = Dash(__name__)
app.layout = Div(
[
Dropdown(
options=[
{"label": Span("red"), "value": "red"},
{"label": Span("yellow"), "value": "yellow"},
{"label": Span("blue"), "value": "blue"},
],
value=["red", "yellow", "blue"],
id="components-label-dropdown",
multi=True,
),
]
)

dash_duo.start_server(app)

dash_duo.find_element("#components-label-dropdown").click()
dash_duo.wait_for_element(".dash-dropdown-options")

# Click on the "yellow" option
yellow_option = dash_duo.find_element(
'.dash-dropdown-option:has(input[value="yellow"])'
)
yellow_option.click()

# After interaction, verify the options render correctly
option_elements = dash_duo.find_elements(".dash-dropdown-option")
rendered_labels = [el.text.strip() for el in option_elements]

assert rendered_labels == ["red", "blue", "yellow"]

assert dash_duo.get_logs() == []
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,17 @@ def update_value(val):

# Attempt to deselect all items. Everything should deselect until we get to
# the last item which cannot be cleared.
selected = dash_duo.find_elements(".dash-dropdown-options input[checked]")
[el.click() for el in selected]
# Click MTL option container
mtl_option = dash_duo.find_element(
'.dash-dropdown-option:has(input[value="MTL"])'
)
mtl_option.click()

# Click SF option container
sf_option = dash_duo.find_element(
'.dash-dropdown-option:has(input[value="SF"])'
)
sf_option.click()

assert dash_duo.find_element("#dropdown-value").text == "SF"

Expand Down