[Dev] Update select ALL logic for controls #1000

nadijagraca · 2025-02-07T11:17:41Z

Description

PR refactors how the "ALL" is handled in the backend for filters and parameters

Current behaviour:

When ALL is selected, a full list of options is sent to the backend.
When ALL is deselected, all values are removed, and an empty list is sent to the backend.
If all values are selected, the ALL checkbox is automatically marked as checked.
When the ALL checkbox is checked, and the user unchecks any value, the ALL checkbox is automatically unchecked.

Screenshot

Notice

I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
- I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
- I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
- I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
- I have not referenced individuals, products or companies in any commits, directly or indirectly.
- I have not added data or restricted code in any commits, directly or indirectly.

"

petar-qb

Thanks for the great work @nadijagraca!! 🚀

We're so close to remove the ALL_OPTION handling from the backend side 😁

petar-qb · 2025-02-07T12:17:38Z

vizro-core/src/vizro/actions/_actions_utils.py

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


Let's leave it like:

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

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

petar-qb · 2025-02-07T12:22:29Z

vizro-core/src/vizro/models/_components/form/_form_utils.py

+    # [{"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]
+


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]

petar-qb · 2025-02-07T12:23:03Z

vizro-core/src/vizro/models/_components/form/_form_utils.py

-        default_value = options[0]["value"]  # type: ignore[index]
-    else:
-        default_value = options[0]
+        dict_options.insert(0, {"label": ALL_OPTION, "value": ALL_OPTION})


Suggested change

dict_options.insert(0, {"label": ALL_OPTION, "value": ALL_OPTION})

dict_options = [{"label": ALL_OPTION, "value": ALL_OPTION}, *dict_options]

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.

petar-qb · 2025-02-07T12:36:10Z

vizro-core/src/vizro/models/_components/form/_form_utils.py

@@ -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`."""


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

petar-qb · 2025-02-07T12:53:12Z

vizro-core/src/vizro/models/_components/form/checklist.py

+                    options=options,
+                    value=self.value if self.value is not None else options,


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,

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 [],

petar-qb · 2025-02-07T13:53:38Z

vizro-core/src/vizro/models/_components/form/dropdown.py

@@ -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(


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

petar-qb · 2025-02-07T13:55:33Z

vizro-core/src/vizro/static/js/models/checklist.js

@@ -0,0 +1,25 @@
+function update_checklist_values(value1 = [], value2 = [], options = []) {


May you rename value1 and value2?

petar-qb · 2025-02-07T13:59:41Z

vizro-core/src/vizro/static/js/models/dropdown.js

+  const options_list = options.map((dict) => dict["value"]);
+  const updated_options = options_list.filter((element) => element !== "ALL");
+
+  if (!value.length) return [[], []];


What case does this line cover?

petar-qb · 2025-02-07T14:00:48Z

vizro-core/src/vizro/static/js/models/dropdown.js

@@ -0,0 +1,40 @@
+// TO-DO: Check if this function triggered when a page is opened


Is this TODO done?

petar-qb · 2025-02-07T14:02:52Z

vizro-core/src/vizro/static/js/models/dropdown.js

+  const isTriggeredByChecklist = triggeredId.includes("_checklist_all");
+  const hasAllSelected = value.includes("ALL");


What is the difference between isTriggeredByChecklist and hasAllSelected?

nadijagraca added 11 commits January 31, 2025 11:25

add backend support for select all

e8a9872

fix tests

4a75b42

remove comment

3bdde34

merge main

ca892a5

intermediate solution

d481425

remove unnecessary brackets"

243b236

"

adding styling changes to dropdown

153cf94

clientside refactor

1785383

minor refactor

5b9d101

merge main

b75a258

clean app file

ed81565

nadijagraca requested review from huong-li-nguyen, Joseph-Perkins, antonymilne, maxschulz-COL and lingyielia as code owners February 7, 2025 11:17

nadijagraca requested a review from petar-qb February 7, 2025 11:43

petar-qb reviewed Feb 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dev] Update select ALL logic for controls #1000

[Dev] Update select ALL logic for controls #1000

nadijagraca commented Feb 7, 2025

petar-qb left a comment

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

petar-qb Feb 7, 2025

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

		@@ -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`."""

		options=options,
		value=self.value if self.value is not None else options,

		@@ -0,0 +1,25 @@
		function update_checklist_values(value1 = [], value2 = [], options = []) {

		@@ -0,0 +1,40 @@
		// TO-DO: Check if this function triggered when a page is opened

		const isTriggeredByChecklist = triggeredId.includes("_checklist_all");
		const hasAllSelected = value.includes("ALL");

[Dev] Update select ALL logic for controls #1000

Are you sure you want to change the base?

[Dev] Update select ALL logic for controls #1000

Conversation

nadijagraca commented Feb 7, 2025

Description

Screenshot

Notice

petar-qb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment