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

[Dev] Update select ALL logic for controls #1000

Open
wants to merge 11 commits into
base: dev/fix_dash_persistence
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<!--
A new scriv changelog fragment.

Uncomment the section that is right (remove the HTML comment wrapper).
-->

<!--
### Highlights ✨

- A bullet item for the Highlights ✨ category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX. ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Removed

- A bullet item for the Removed category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX. ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Added

- A bullet item for the Added category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX. ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Changed

- A bullet item for the Changed category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX. ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Deprecated

- A bullet item for the Deprecated category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX. ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Fixed

- A bullet item for the Fixed category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX. ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Security

- A bullet item for the Security category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX. ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
112 changes: 44 additions & 68 deletions vizro-core/examples/scratch_dev/app.py
Original file line number Diff line number Diff line change
@@ -1,78 +1,54 @@
"""Dev app to try things out."""

import pandas as pd
import vizro.plotly.express as px
import vizro.models as vm
from vizro import Vizro

# For more information, refer to the API reference for kpi_card and kpi_card_reference
from vizro.figures import kpi_card, kpi_card_reference

df_kpi = pd.DataFrame({"Actual": [100, 200, 700], "Reference": [100, 300, 500], "Category": ["A", "B", "C"]})

example_cards = [
kpi_card(data_frame=df_kpi, value_column="Actual", title="KPI with value"),
kpi_card(data_frame=df_kpi, value_column="Actual", title="KPI with aggregation", agg_func="median"),
kpi_card(
data_frame=df_kpi,
value_column="Actual",
title="KPI with formatting",
value_format="${value:.2f}",
),
kpi_card(
data_frame=df_kpi,
value_column="Actual",
title="KPI with icon",
icon="shopping_cart",
),
]

example_reference_cards = [
kpi_card_reference(
data_frame=df_kpi,
value_column="Actual",
reference_column="Reference",
title="KPI reference (pos)",
),
kpi_card_reference(
data_frame=df_kpi,
value_column="Actual",
reference_column="Reference",
agg_func="median",
title="KPI reference (neg)",
),
kpi_card_reference(
data_frame=df_kpi,
value_column="Actual",
reference_column="Reference",
title="KPI reference with formatting",
value_format="{value:.2f}€",
reference_format="{delta:+.2f}€ vs. last year ({reference:.2f}€)",
),
kpi_card_reference(
data_frame=df_kpi,
value_column="Actual",
reference_column="Reference",
title="KPI reference with icon",
icon="shopping_cart",
),
kpi_card_reference(
data_frame=df_kpi,
value_column="Actual",
reference_column="Reference",
title="KPI reference (reverse color)",
reverse_color=True,
),
]

# Create a layout with four rows and columns. The KPI cards are positioned in the first nine cells, while the remaining cells are empty.
page = vm.Page(
title="KPI cards",
layout=vm.Layout(grid=[[0, 1, 2, 3], [4, 5, 6, 7], [8, -1, -1, -1], [-1, -1, -1, -1]]),
components=[vm.Figure(figure=figure) for figure in example_cards + example_reference_cards],
controls=[vm.Filter(column="Category")],
from vizro.tables import dash_ag_grid


df = px.data.gapminder()

first_page = vm.Page(
title="First Page",
layout=vm.Layout(grid=[[0, 0], [1, 1], [1, 1], [1, 1]]),
components=[
vm.Card(
text="""
# First dashboard page
This pages shows the inclusion of markdown text in a page and how components
can be structured using Layout.
""",
),
vm.AgGrid(
figure=dash_ag_grid(data_frame=df, dashGridOptions={"pagination": True}),
title="Gapminder Data Insights",
header="""#### An Interactive Exploration of Global Health, Wealth, and Population""",
footer="""SOURCE: **Plotly gapminder data set, 2024**""",
),
],
controls=[
# vm.Filter(column="continent", selector=vm.Checklist()),
vm.Filter(
column="country",
# column="continent",
selector=vm.Dropdown(
# options=[
# {"label": "EUROPE", "value": "Europe"},
# {"label": "AFRICA", "value": "Africa"},
# {"label": "ASIA", "value": "Asia"},
# {"label": "AMERICAS", "value": "Americas"},
# {"label": "OCEANIA", "value": "Oceania"},
# ],
# value=["Europe", "Africa", "Asia", "Americas", "Oceania"],
# value="Europe",
# multi=False,
),
),
],
)

dashboard = vm.Dashboard(pages=[page])
dashboard = vm.Dashboard(pages=[first_page])

if __name__ == "__main__":
Vizro().build(dashboard).run()
7 changes: 0 additions & 7 deletions vizro-core/src/vizro/actions/_actions_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,6 @@ def _apply_filter_controls(
selector_actions = _get_component_actions(model_manager[ctd["id"]])

for action in selector_actions:
if (
action.function._function.__name__ != "_filter"
or target not in action.function["targets"]
or ALL_OPTION in selector_value
):
continue

Comment on lines -75 to -81
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's leave it like:

            if (
                action.function._function.__name__ != "_filter"
                or target not in action.function["targets"]
            ):
                continue

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also adjust the code from the line #192.

_filter_function = action.function["filter_function"]
_filter_column = action.function["filter_column"]
_filter_value = selector_value
Expand Down
22 changes: 11 additions & 11 deletions vizro-core/src/vizro/models/_components/form/_form_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@

def get_options_and_default(options: OptionsType, multi: bool = False) -> tuple[OptionsType, SingleValueType]:
"""Gets list of full options and default value based on user input type of `options`."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should highlight in the docstring or maybe even in the function name that returned options are in list[dict] format.

# [{"label": "Option 1", "value": "Option 1"}, {"label": "Option 2", "value": "Option 2"}]
dict_options = [
option if isinstance(option, dict) else {"label": str(option), "value": option} for option in options
]

# ["Option 1", "Option 2", ...]
all_values = [dict_option["value"] for dict_option in dict_options]
default_value = all_values if multi else all_values[0]

Comment on lines +14 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# [{"label": "Option 1", "value": "Option 1"}, {"label": "Option 2", "value": "Option 2"}]
dict_options = [
option if isinstance(option, dict) else {"label": str(option), "value": option} for option in options
]
# ["Option 1", "Option 2", ...]
all_values = [dict_option["value"] for dict_option in dict_options]
default_value = all_values if multi else all_values[0]
dict_options = [
option if isinstance(option, dict) else {"label": str(option), "value": option} for option in options
]
list_value = [dict_option["value"] for dict_option in dict_options]
default_value = list_value if multi else list_value[0]

if multi:
if all(isinstance(option, dict) for option in options):
options = [{"label": ALL_OPTION, "value": ALL_OPTION}, *options]
else:
options = [ALL_OPTION, *options]

if all(isinstance(option, dict) for option in options):
# Each option is a OptionsDictType
default_value = options[0]["value"] # type: ignore[index]
else:
default_value = options[0]
dict_options.insert(0, {"label": ALL_OPTION, "value": ALL_OPTION})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dict_options.insert(0, {"label": ALL_OPTION, "value": ALL_OPTION})
dict_options = [{"label": ALL_OPTION, "value": ALL_OPTION}, *dict_options]

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I think we should remove handling of ALL_OPTION from this function.

It just adds a confusion. ALL_OPTION is added only for vm.Dropdown(multi=True) and vm.Checklist, and then, both Checklist and Dropdown apply different and custom logic on top of that in their __call__ and helper functions. As those two classes they treat the ALL_OPTION differently, we might have try to remove inserting the ALL_OPTION in this function.


return options, default_value
return dict_options, default_value


# Utils for validators
Expand Down
28 changes: 23 additions & 5 deletions vizro-core/src/vizro/models/_components/form/checklist.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import Annotated, Literal, Optional

import dash_bootstrap_components as dbc
from dash import html
from dash import ClientsideFunction, Input, Output, State, clientside_callback, html
from pydantic import AfterValidator, Field, PrivateAttr, model_validator
from pydantic.functional_serializers import PlainSerializer

Expand Down Expand Up @@ -50,15 +50,33 @@ class Checklist(VizroBaseModel):
_validate_options = model_validator(mode="before")(validate_options_dict)

def __call__(self, options):
full_options, default_value = get_options_and_default(options=options, multi=True)
output = [Output(f"{self.id}", "value"), Output(f"{self.id}_select_all", "value")]
inputs = [
Input(f"{self.id}_select_all", "value"),
Input(f"{self.id}", "value"),
State(f"{self.id}", "options"),
]

clientside_callback(
ClientsideFunction(namespace="checklist", function_name="update_checklist_values"),
output=output,
inputs=inputs,
)

return html.Fieldset(
children=[
html.Legend(children=self.title, className="form-label") if self.title else None,
dbc.Checklist(
id=f"{self.id}_select_all",
options=["ALL"],
value=["ALL"] if self.value == self.options or self.value is None else [],
Comment on lines +71 to +72
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use ALL_OPTIONS instead of "ALL" everywhere.

persistence=True,
persistence_type="session",
),
dbc.Checklist(
id=self.id,
options=full_options,
value=self.value if self.value is not None else [default_value],
options=options,
value=self.value if self.value is not None else options,
Comment on lines +78 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

If we remove ALL_OPTION handling from the get_options_and_default (as I mentioned in the previous comment), then this could look like:

Suggested change
options=options,
value=self.value if self.value is not None else options,
options=full_options # (or dict_options if we rename it then),
value=self.value if self.value else default_value,

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even final_value can be calculated before as final_value = self.value if self.value else default_value, and then reuse final_value here and above as: value=["ALL"] if final_value == full_options esle [],

persistence=True,
persistence_type="session",
),
Expand All @@ -68,7 +86,7 @@ def __call__(self, options):
def _build_dynamic_placeholder(self):
if self.value is None:
_, default_value = get_options_and_default(self.options, multi=True)
self.value = [default_value] # type: ignore[assignment]
self.value = default_value # type: ignore[assignment]
Comment on lines -71 to +89
Copy link
Contributor

Choose a reason for hiding this comment

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

It's nice to see this 🤩


return self.__call__(self.options)

Expand Down
54 changes: 43 additions & 11 deletions vizro-core/src/vizro/models/_components/form/dropdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from typing import Annotated, Literal, Optional, Union, cast

import dash_bootstrap_components as dbc
from dash import dcc, html
from dash import ClientsideFunction, Input, Output, State, clientside_callback, dcc, html
from pydantic import AfterValidator, Field, PrivateAttr, StrictBool, ValidationInfo, model_validator
from pydantic.functional_serializers import PlainSerializer

Expand Down Expand Up @@ -44,16 +44,31 @@ def validate_multi(multi, info: ValidationInfo):
return multi


def _add_select_all_option(full_options: OptionsType) -> OptionsType:
def _add_select_all_option(
Copy link
Contributor

Choose a reason for hiding this comment

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

This function looks like also could be simplified a bit if we decide to remove ALL_OPTION handling from the get_options_and_default function

full_options: OptionsType, component_id: str, value: Optional[Union[SingleValueType, MultiValueType]]
) -> OptionsType:
"""Adds a 'Select All' option to the list of options."""
# TODO: Move option to dictionary conversion within `get_options_and_default` function as here: https://github.com/mckinsey/vizro/pull/961#discussion_r1923356781
options_dict = [
cast(OptionsDictType, {"label": option, "value": option}) if not isinstance(option, dict) else option
for option in full_options
]

options_dict[0] = {"label": html.Div(["ALL"]), "value": "ALL"}
return options_dict
checklist_value = (
["ALL"] if value is None or (isinstance(value, list) and len(value) == len(full_options) - 1) else []
)
full_options = cast(list[OptionsDictType], full_options)
full_options[0] = {
"label": html.Div(
[
dcc.Checklist(
options=[{"label": "", "value": "ALL"}],
value=checklist_value,
id=f"{component_id}_checklist_all",
persistence=True,
persistence_type="session",
),
html.Span("ALL"),
],
className="checklist-dropdown-div",
),
"value": "ALL",
}
return full_options


class Dropdown(VizroBaseModel):
Expand Down Expand Up @@ -106,9 +121,26 @@ class Dropdown(VizroBaseModel):
_validate_options = model_validator(mode="before")(validate_options_dict)

def __call__(self, options):
if self.multi:
output = [Output(f"{self.id}", "value"), Output(f"{self.id}_checklist_all", "value")]
inputs = [
Input(f"{self.id}", "value"),
Input(f"{self.id}_checklist_all", "value"),
State(f"{self.id}", "options"),
]

clientside_callback(
ClientsideFunction(namespace="dropdown", function_name="update_dropdown_values"),
output=output,
inputs=inputs,
)
Comment on lines +124 to +136
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's create a callback without helper variables. I also recommend to stay consistent and to change the order of the inputs and outputs to: (same for dropdown client-side callback):

  1. {self.id}_checklist_all
  2. {self.id}
  3. other inputs/outputs
Suggested change
if self.multi:
output = [Output(f"{self.id}", "value"), Output(f"{self.id}_checklist_all", "value")]
inputs = [
Input(f"{self.id}", "value"),
Input(f"{self.id}_checklist_all", "value"),
State(f"{self.id}", "options"),
]
clientside_callback(
ClientsideFunction(namespace="dropdown", function_name="update_dropdown_values"),
output=output,
inputs=inputs,
)
if self.multi:
clientside_callback(
ClientsideFunction(namespace="dropdown", function_name="update_dropdown_values"),
output=[
Output(f"{self.id}", "value"),
Output(f"{self.id}_checklist_all", "value"),
],
inputs=[
Input(f"{self.id}", "value"),
Input(f"{self.id}_checklist_all", "value"),
State(f"{self.id}", "options"),
],
)

full_options, default_value = get_options_and_default(options=options, multi=self.multi)
option_height = _calculate_option_height(full_options)
altered_options = _add_select_all_option(full_options=full_options) if self.multi else full_options
altered_options = (
_add_select_all_option(full_options=full_options, component_id=self.id, value=self.value)
if self.multi
else full_options
)

return html.Div(
children=[
Expand Down
3 changes: 3 additions & 0 deletions vizro-core/src/vizro/static/css/checklist.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[id$="_select_all"] {
margin-bottom: 12px;
}
10 changes: 9 additions & 1 deletion vizro-core/src/vizro/static/css/dropdown.css
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@

/* Border on focus */
#dashboard-container .is-focused:not(.is-open) > .Select-control {
box-shadow: 0 0 0 2px var(--focus) inset;
box-shadow: 0 0 0 2px var(--focus);
}

/* Single-select dropdown only ------------------- */
Expand Down Expand Up @@ -150,6 +150,8 @@ wrapper **/
display: flex;
flex-wrap: wrap;
gap: 4px;
max-height: 90px;
overflow: auto;
padding: 4px 8px;
}

Expand All @@ -160,3 +162,9 @@ wrapper **/
padding: 0;
padding-left: 0.5rem;
}

.checklist-dropdown-div {
display: flex;
flex-direction: row;
gap: 8px;
}
Loading