From 3033abd9ee9f7b42b94f156c596a29be594411cf Mon Sep 17 00:00:00 2001 From: nadijagraca Date: Wed, 27 Sep 2023 15:25:17 +0200 Subject: [PATCH 01/18] fixing accordion item highlight bug --- .../vizro/models/_navigation/_accordion.py | 20 +++++++++---------- .../vizro/models/_navigation/navigation.py | 4 ++-- vizro-core/src/vizro/models/_page.py | 4 ++-- vizro-core/src/vizro/static/css/accordion.css | 11 ++++++++-- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/vizro-core/src/vizro/models/_navigation/_accordion.py b/vizro-core/src/vizro/models/_navigation/_accordion.py index e2b60687b..32d45e00c 100644 --- a/vizro-core/src/vizro/models/_navigation/_accordion.py +++ b/vizro-core/src/vizro/models/_navigation/_accordion.py @@ -26,17 +26,17 @@ class Accordion(VizroBaseModel): _validate_pages = validator("pages", allow_reuse=True, always=True)(_validate_pages) @_log_call - def build(self): + def build(self, page_id): if self.pages: - return self._create_accordion() - return self._create_default_accordion() + return self._create_accordion(page_id=page_id) + return self._create_default_accordion(page_id=page_id) - def _create_accordion_buttons(self, accordion_pages): + def _create_accordion_buttons(self, accordion_pages, page_id): return [ dbc.Button( children=[page["name"]], key=page["relative_path"], - className="accordion-item-button", + className="accordion-item-button-active" if page_id == page["name"] else "accordion-item-button", href=page["relative_path"], ) for page in dash.page_registry.values() @@ -51,12 +51,12 @@ def _create_accordion_item(self, accordion_buttons, title=ACCORDION_TITLE): class_name="accordion_item", ) - def _create_default_accordion(self): + def _create_default_accordion(self, page_id): accordion_buttons = [ dbc.Button( children=[page["name"]], key=page["relative_path"], - className="accordion-item-button", + className="accordion-item-button-active" if page_id == page["name"] else "accordion-item-button", href=page["relative_path"], ) for page in dash.page_registry.values() @@ -84,16 +84,16 @@ def _create_default_accordion(self): id=f"{self.id}_outer", ) - def _create_accordion(self): + def _create_accordion(self, page_id): accordion_items = [] if isinstance(self.pages, dict): for title, accordion_pages in self.pages.items(): - accordion_buttons = self._create_accordion_buttons(accordion_pages=accordion_pages) + accordion_buttons = self._create_accordion_buttons(accordion_pages=accordion_pages, page_id=page_id) accordion_items.append(self._create_accordion_item(accordion_buttons=accordion_buttons, title=title)) if isinstance(self.pages, list): - accordion_buttons = self._create_accordion_buttons(accordion_pages=self.pages) + accordion_buttons = self._create_accordion_buttons(accordion_pages=self.pages, page_id=page_id) accordion_items.append(self._create_accordion_item(accordion_buttons=accordion_buttons)) if len(accordion_buttons) == len(accordion_items) == 1: diff --git a/vizro-core/src/vizro/models/_navigation/navigation.py b/vizro-core/src/vizro/models/_navigation/navigation.py index 756fe8d9e..41245e55b 100644 --- a/vizro-core/src/vizro/models/_navigation/navigation.py +++ b/vizro-core/src/vizro/models/_navigation/navigation.py @@ -72,5 +72,5 @@ def _set_selector(self): self._selector = Accordion(pages=self.pages) @_log_call - def build(self): - return self._selector.build() + def build(self, page_id): + return self._selector.build(page_id=page_id) diff --git a/vizro-core/src/vizro/models/_page.py b/vizro-core/src/vizro/models/_page.py index 5906a26f9..912d62e89 100644 --- a/vizro-core/src/vizro/models/_page.py +++ b/vizro-core/src/vizro/models/_page.py @@ -172,9 +172,9 @@ def _create_nav_panel(self): _, dashboard = next(model_manager._items_with_type(Dashboard)) if dashboard.navigation: - return dashboard.navigation.build() + return dashboard.navigation.build(page_id=self.id) - return Accordion().build() + return Accordion().build(page_id=self.id) def _create_component_container(self, components_content): component_container = html.Div( diff --git a/vizro-core/src/vizro/static/css/accordion.css b/vizro-core/src/vizro/static/css/accordion.css index 1ed9eb28a..9ba7e2ff7 100644 --- a/vizro-core/src/vizro/static/css/accordion.css +++ b/vizro-core/src/vizro/static/css/accordion.css @@ -48,7 +48,7 @@ .accordion-button:not(.collapsed) { color: var(--text-primary); - background-color: var(--state-overlays-selected); + /*background-color: var(--state-overlays-selected); */ } .accordion-button:not(.collapsed)::after { @@ -88,6 +88,7 @@ background-color: inherit; width: 100%; border-bottom: 1px solid var(--border-subtle-alpha-01); + gap: var(--spacing-01); } .accordion_item:last-child { @@ -102,7 +103,8 @@ gap: var(--spacing-01); } -div.page_container .accordion-item-button { +div.page_container .accordion-item-button, +.accordion-item-button-active{ background-color: inherit; width: 100%; display: flex; @@ -113,6 +115,11 @@ div.page_container .accordion-item-button { line-height: var(--text-size-03); } +.accordion-item-button-active { + background-color: var(--state-overlays-selected); + color: var(--text-primary); +} + /* * Since the text inside a accordion btn is very specific * Adding important to increase the specificity From b4cbf9fbf29a8331f46b54437b38bd3796a13656 Mon Sep 17 00:00:00 2001 From: nadijagraca Date: Thu, 28 Sep 2023 14:16:34 +0200 Subject: [PATCH 02/18] removing background color highlight from selected accordion, aligning accordion title with page title --- vizro-core/src/vizro/models/_navigation/_accordion.py | 1 + vizro-core/src/vizro/static/css/accordion.css | 1 - vizro-core/src/vizro/static/css/layout.css | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vizro-core/src/vizro/models/_navigation/_accordion.py b/vizro-core/src/vizro/models/_navigation/_accordion.py index 32d45e00c..758fce943 100644 --- a/vizro-core/src/vizro/models/_navigation/_accordion.py +++ b/vizro-core/src/vizro/models/_navigation/_accordion.py @@ -107,6 +107,7 @@ def _create_accordion(self, page_id): class_name="accordion", persistence=True, persistence_type="session", + always_open=True, ), html.Div(className="keyline"), ], diff --git a/vizro-core/src/vizro/static/css/accordion.css b/vizro-core/src/vizro/static/css/accordion.css index 9ba7e2ff7..1f0a984bd 100644 --- a/vizro-core/src/vizro/static/css/accordion.css +++ b/vizro-core/src/vizro/static/css/accordion.css @@ -48,7 +48,6 @@ .accordion-button:not(.collapsed) { color: var(--text-primary); - /*background-color: var(--state-overlays-selected); */ } .accordion-button:not(.collapsed)::after { diff --git a/vizro-core/src/vizro/static/css/layout.css b/vizro-core/src/vizro/static/css/layout.css index 334a96349..dcdd82efb 100644 --- a/vizro-core/src/vizro/static/css/layout.css +++ b/vizro-core/src/vizro/static/css/layout.css @@ -11,7 +11,7 @@ display: flex; flex: 0 0 auto; flex-direction: column; - padding: 40px 32px 0 32px; + padding: 30px 32px 0 32px; scrollbar-gutter: stable; width: 352px; overflow: auto; From f630b505081d54e103077c35f6918aa042d7689a Mon Sep 17 00:00:00 2001 From: nadijagraca Date: Thu, 28 Sep 2023 14:42:08 +0200 Subject: [PATCH 03/18] added changelog --- ...c_graca_highlight_active_accordion_item.md | 41 +++++++++++++++++++ vizro-core/src/vizro/static/css/accordion.css | 1 - 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 vizro-core/changelog.d/20230928_143738_nadija_ratkusic_graca_highlight_active_accordion_item.md diff --git a/vizro-core/changelog.d/20230928_143738_nadija_ratkusic_graca_highlight_active_accordion_item.md b/vizro-core/changelog.d/20230928_143738_nadija_ratkusic_graca_highlight_active_accordion_item.md new file mode 100644 index 000000000..b598a9e40 --- /dev/null +++ b/vizro-core/changelog.d/20230928_143738_nadija_ratkusic_graca_highlight_active_accordion_item.md @@ -0,0 +1,41 @@ + + + + + + + +### Fixed + +- Fixed highlighting for active accordion item and arranged a setup to maintain the expanded state of the parent accordion item when page is active. ([#74](https://github.com/mckinsey/vizro/pull/74)) + + diff --git a/vizro-core/src/vizro/static/css/accordion.css b/vizro-core/src/vizro/static/css/accordion.css index 1f0a984bd..3e29fc19e 100644 --- a/vizro-core/src/vizro/static/css/accordion.css +++ b/vizro-core/src/vizro/static/css/accordion.css @@ -87,7 +87,6 @@ background-color: inherit; width: 100%; border-bottom: 1px solid var(--border-subtle-alpha-01); - gap: var(--spacing-01); } .accordion_item:last-child { From d843b1d8b5e04bd6b63bcc415f4f7729da8424a2 Mon Sep 17 00:00:00 2001 From: nadijagraca Date: Thu, 28 Sep 2023 14:47:08 +0200 Subject: [PATCH 04/18] minor linting fixes' --- vizro-core/src/vizro/static/css/accordion.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vizro-core/src/vizro/static/css/accordion.css b/vizro-core/src/vizro/static/css/accordion.css index 3e29fc19e..cb7f117c5 100644 --- a/vizro-core/src/vizro/static/css/accordion.css +++ b/vizro-core/src/vizro/static/css/accordion.css @@ -102,7 +102,7 @@ } div.page_container .accordion-item-button, -.accordion-item-button-active{ +.accordion-item-button-active { background-color: inherit; width: 100%; display: flex; From 417ca3c0a481ac38c973c64b06e5fbcab7a32521 Mon Sep 17 00:00:00 2001 From: nadijagraca Date: Thu, 28 Sep 2023 15:54:26 +0200 Subject: [PATCH 05/18] fixed tests --- vizro-core/src/vizro/models/_navigation/_accordion.py | 1 + .../tests/unit/vizro/models/_navigation/conftest.py | 6 ++++-- .../tests/unit/vizro/models/_navigation/test_accordion.py | 8 ++++---- .../unit/vizro/models/_navigation/test_navigation.py | 4 ++-- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/vizro-core/src/vizro/models/_navigation/_accordion.py b/vizro-core/src/vizro/models/_navigation/_accordion.py index 758fce943..370124efb 100644 --- a/vizro-core/src/vizro/models/_navigation/_accordion.py +++ b/vizro-core/src/vizro/models/_navigation/_accordion.py @@ -77,6 +77,7 @@ def _create_default_accordion(self, page_id): class_name="accordion", persistence=True, persistence_type="session", + always_open=True, ), html.Div(className="keyline"), ], diff --git a/vizro-core/tests/unit/vizro/models/_navigation/conftest.py b/vizro-core/tests/unit/vizro/models/_navigation/conftest.py index 619eb1fa4..76eb4f267 100644 --- a/vizro-core/tests/unit/vizro/models/_navigation/conftest.py +++ b/vizro-core/tests/unit/vizro/models/_navigation/conftest.py @@ -26,7 +26,7 @@ def accordion_from_page_as_list(): accordion_buttons = [ dbc.Button( children=["Page 1"], - className="accordion-item-button", + className="accordion-item-button-active", key="/", href="/", ), @@ -52,6 +52,7 @@ def accordion_from_page_as_list(): class_name="accordion", persistence=True, persistence_type="session", + always_open=True, ), html.Div(className="keyline"), ], @@ -65,7 +66,7 @@ def accordion_from_page_as_list(): def accordion_from_pages_as_dict(): accordion_items = [ dbc.AccordionItem( - children=[dbc.Button(children=["Page 1"], className="accordion-item-button", key="/", href="/")], + children=[dbc.Button(children=["Page 1"], className="accordion-item-button-active", key="/", href="/")], title="PAGE 1", class_name="accordion_item", ), @@ -85,6 +86,7 @@ def accordion_from_pages_as_dict(): class_name="accordion", persistence=True, persistence_type="session", + always_open=True, ), html.Div(className="keyline"), ], diff --git a/vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py b/vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py index d23506890..7bb682b78 100644 --- a/vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py +++ b/vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py @@ -43,25 +43,25 @@ class TestAccordionBuild: @pytest.mark.parametrize("pages", [["Page 1", "Page 2"], None]) def test_accordion_build_default(self, pages, accordion_from_page_as_list): - accordion = Accordion(pages=pages, id="accordion_list").build() + accordion = Accordion(pages=pages, id="accordion_list").build(page_id="Page 1") result = json.loads(json.dumps(accordion, cls=plotly.utils.PlotlyJSONEncoder)) expected = json.loads(json.dumps(accordion_from_page_as_list, cls=plotly.utils.PlotlyJSONEncoder)) assert result == expected def test_accordion_build_pages_as_list(self, pages_as_list, accordion_from_page_as_list): - accordion = Accordion(pages=pages_as_list, id="accordion_list").build() + accordion = Accordion(pages=pages_as_list, id="accordion_list").build(page_id="Page 1") result = json.loads(json.dumps(accordion, cls=plotly.utils.PlotlyJSONEncoder)) expected = json.loads(json.dumps(accordion_from_page_as_list, cls=plotly.utils.PlotlyJSONEncoder)) assert result == expected def test_accordion_build_pages_as_dict(self, pages_as_dict, accordion_from_pages_as_dict): - accordion = Accordion(pages=pages_as_dict, id="accordion_dict").build() + accordion = Accordion(pages=pages_as_dict, id="accordion_dict").build(page_id="Page 1") result = json.loads(json.dumps(accordion, cls=plotly.utils.PlotlyJSONEncoder)) expected = json.loads(json.dumps(accordion_from_pages_as_dict, cls=plotly.utils.PlotlyJSONEncoder)) assert result == expected def test_accordion_build_single_page_accordion(self): - accordion = Accordion(pages=["Page 1"], id="single_accordion").build() + accordion = Accordion(pages=["Page 1"], id="single_accordion").build(page_id="Page 1") assert accordion is None def test_navigation_not_all_pages_included(self, dashboard_build): diff --git a/vizro-core/tests/unit/vizro/models/_navigation/test_navigation.py b/vizro-core/tests/unit/vizro/models/_navigation/test_navigation.py index dce83ddbb..b28ab5e93 100644 --- a/vizro-core/tests/unit/vizro/models/_navigation/test_navigation.py +++ b/vizro-core/tests/unit/vizro/models/_navigation/test_navigation.py @@ -56,6 +56,6 @@ def test_navigation_build(self, pages): accordion = Accordion(pages=pages) navigation._selector.id = accordion.id - result = json.loads(json.dumps(navigation.build(), cls=plotly.utils.PlotlyJSONEncoder)) - expected = json.loads(json.dumps(accordion.build(), cls=plotly.utils.PlotlyJSONEncoder)) + result = json.loads(json.dumps(navigation.build(page_id="Page 1"), cls=plotly.utils.PlotlyJSONEncoder)) + expected = json.loads(json.dumps(accordion.build(page_id="Page 1"), cls=plotly.utils.PlotlyJSONEncoder)) assert result == expected From e6307aa7f72e792eb94232620027d50a64602df5 Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Mon, 2 Oct 2023 13:18:30 +0200 Subject: [PATCH 06/18] Lint --- vizro-core/src/vizro/models/_navigation/_accordion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vizro-core/src/vizro/models/_navigation/_accordion.py b/vizro-core/src/vizro/models/_navigation/_accordion.py index 1b9568959..6eb945325 100644 --- a/vizro-core/src/vizro/models/_navigation/_accordion.py +++ b/vizro-core/src/vizro/models/_navigation/_accordion.py @@ -29,7 +29,7 @@ class Accordion(VizroBaseModel): def build(self, page_id): return self._create_accordion(page_id=page_id) - def _create_accordion_buttons(self, pages): + def _create_accordion_buttons(self, pages, page_id): """Creates a button for each provided page.""" # TODO: Better if we loop through pages from MM so the Accordion.build does not depend on dashboard build. # However, this would require that only pages used in the Dashboard are registered in the MM. From 9fecf3ca386c011f941d90c0bf5454fdeabe861a Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Mon, 2 Oct 2023 13:23:59 +0200 Subject: [PATCH 07/18] Replace active class with active param --- ...ratkusic_graca_highlight_active_accordion_item.md | 2 +- .../src/vizro/models/_navigation/_accordion.py | 3 ++- vizro-core/src/vizro/static/css/accordion.css | 5 ++--- .../tests/unit/vizro/models/_navigation/conftest.py | 12 +++++++++--- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/vizro-core/changelog.d/20230928_143738_nadija_ratkusic_graca_highlight_active_accordion_item.md b/vizro-core/changelog.d/20230928_143738_nadija_ratkusic_graca_highlight_active_accordion_item.md index b598a9e40..dc417ce13 100644 --- a/vizro-core/changelog.d/20230928_143738_nadija_ratkusic_graca_highlight_active_accordion_item.md +++ b/vizro-core/changelog.d/20230928_143738_nadija_ratkusic_graca_highlight_active_accordion_item.md @@ -31,7 +31,7 @@ Uncomment the section that is right (remove the HTML comment wrapper). ### Fixed -- Fixed highlighting for active accordion item and arranged a setup to maintain the expanded state of the parent accordion item when page is active. ([#74](https://github.com/mckinsey/vizro/pull/74)) +- Add highlighting to selected page accordion button and change the collapsible default behavior of accordion ([#74](https://github.com/mckinsey/vizro/pull/74)) - - +- Add validator for `dashboard.navigation` to default to `Navigation()` if not provided ([#74](https://github.com/mckinsey/vizro/pull/74)) ### Changed -- Move creation of `dash.page_registry` to `Dashboard.pre-build` ([#74](https://github.com/mckinsey/vizro/pull/74)) +- Move creation of `dash.page_registry` to `Dashboard.pre_build` ([#74](https://github.com/mckinsey/vizro/pull/74)) +- Change the collapsible default behavior and highlighting color of the selected accordion group ([#74](https://github.com/mckinsey/vizro/pull/74))