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

Enable highlighting of active page button and refactor accordion.build() #74

Merged
merged 24 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
3033abd
fixing accordion item highlight bug
nadijagraca Sep 27, 2023
b4cbf9f
removing background color highlight from selected accordion, aligning…
nadijagraca Sep 28, 2023
f630b50
added changelog
nadijagraca Sep 28, 2023
d843b1d
minor linting fixes'
nadijagraca Sep 28, 2023
417ca3c
fixed tests
nadijagraca Sep 28, 2023
19983a8
Merge branch 'main' into vizro/bug/highlight_active_accordion_item
huong-li-nguyen Oct 2, 2023
e6307aa
Lint
huong-li-nguyen Oct 2, 2023
9fecf3c
Replace active class with active param
huong-li-nguyen Oct 2, 2023
8cd1bfc
Merge branch 'main' into vizro/bug/highlight_active_accordion_item
huong-li-nguyen Oct 2, 2023
5768665
Merge branch 'main' into vizro/bug/highlight_active_accordion_item
huong-li-nguyen Oct 3, 2023
901c7f2
Merge branch 'main' into vizro/bug/highlight_active_accordion_item
huong-li-nguyen Oct 3, 2023
4bdca79
Refactor code
huong-li-nguyen Oct 3, 2023
e320bb8
Fix tests
huong-li-nguyen Oct 3, 2023
ca5e675
Lint
huong-li-nguyen Oct 3, 2023
56ebe70
Merge branch 'main' into vizro/bug/highlight_active_accordion_item
huong-li-nguyen Oct 3, 2023
5081c89
Move page registering to Dashboard.pre_build
huong-li-nguyen Oct 3, 2023
6401a26
Update changelog
huong-li-nguyen Oct 3, 2023
a9dc868
Remove Accordion.build from page.build
huong-li-nguyen Oct 3, 2023
e3851c3
Remove navigation.pre_build from page and fix tests
huong-li-nguyen Oct 3, 2023
203975f
Update changelog
huong-li-nguyen Oct 4, 2023
2fa3429
PR comments
huong-li-nguyen Oct 4, 2023
947cdaf
Merge branch 'main' into vizro/bug/highlight_active_accordion_item
huong-li-nguyen Oct 4, 2023
a316e42
Fix access to dash.page_registry
huong-li-nguyen Oct 4, 2023
bb8d9f0
Update KeyError
huong-li-nguyen Oct 4, 2023
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,41 @@
<!--
A new scriv changelog fragment.

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

<!--
### Removed

- A bullet item for the Removed category.

-->
<!--
### Added

- A bullet item for the Added category.

-->
<!--
### Changed

- A bullet item for the Changed category.

-->
<!--
### Deprecated

- A bullet item for the Deprecated category.

-->

### Fixed

- Add highlighting to selected page accordion button and change the collapsible default behavior of accordion ([#74](https://github.com/mckinsey/vizro/pull/74))
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved

<!--
### Security

- A bullet item for the Security category.

-->
14 changes: 8 additions & 6 deletions vizro-core/src/vizro/models/_navigation/_accordion.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ class Accordion(VizroBaseModel):
_validate_pages = validator("pages", allow_reuse=True, always=True)(_validate_pages)

@_log_call
def build(self):
return self._create_accordion()
def build(self, page_id):
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand All @@ -39,6 +39,7 @@ def _create_accordion_buttons(self, pages):
children=[page["name"]],
key=page["relative_path"],
className="accordion-item-button",
active=True if page_id == page["name"] else False,
href=page["relative_path"],
)
for page in dash.page_registry.values()
Expand Down Expand Up @@ -66,24 +67,25 @@ def _get_accordion_container(self, accordion_items, accordion_buttons):
class_name="accordion",
persistence=True,
persistence_type="session",
always_open=True,
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved
),
html.Hr(),
],
className="nav_panel",
id=f"{self.id}_outer",
)

def _create_accordion(self):
def _create_accordion(self, page_id):
"""Creates a custom accordion only with user-provided pages."""
accordion_items = []
if isinstance(self.pages, dict):
for page_group, page_members in self.pages.items():
accordion_buttons = self._create_accordion_buttons(pages=page_members)
accordion_buttons = self._create_accordion_buttons(pages=page_members, page_id=page_id)
accordion_items.append(
self._create_accordion_item(accordion_buttons=accordion_buttons, title=page_group)
)

if isinstance(self.pages, list):
accordion_buttons = self._create_accordion_buttons(pages=self.pages)
accordion_buttons = self._create_accordion_buttons(pages=self.pages, page_id=page_id)
accordion_items.append(self._create_accordion_item(accordion_buttons=accordion_buttons))
return self._get_accordion_container(accordion_items=accordion_items, accordion_buttons=accordion_buttons)
4 changes: 2 additions & 2 deletions vizro-core/src/vizro/models/_navigation/navigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,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)
4 changes: 2 additions & 2 deletions vizro-core/src/vizro/models/_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,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(
Expand Down
6 changes: 5 additions & 1 deletion vizro-core/src/vizro/static/css/accordion.css
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@

.accordion-button:not(.collapsed) {
color: var(--text-primary);
background-color: var(--state-overlays-selected);
}

.accordion-button:not(.collapsed)::after {
Expand Down Expand Up @@ -113,6 +112,11 @@ div.page_container .accordion-item-button {
line-height: var(--text-size-03);
}

.accordion-item-button.btn.btn-primary.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
Expand Down
2 changes: 1 addition & 1 deletion vizro-core/src/vizro/static/css/layout.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 10 additions & 2 deletions vizro-core/tests/unit/vizro/models/_navigation/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ def accordion_from_page_as_list():
dbc.Button(
children=["Page 1"],
className="accordion-item-button",
active=True,
key="/",
href="/",
),
dbc.Button(
children=["Page 2"],
className="accordion-item-button",
active=False,
key="/page-2",
href="/page-2",
),
Expand All @@ -48,6 +50,7 @@ def accordion_from_page_as_list():
class_name="accordion",
persistence=True,
persistence_type="session",
always_open=True,
),
html.Hr(),
],
Expand All @@ -61,13 +64,17 @@ 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=True, key="/", href="/")
],
title="PAGE 1",
class_name="accordion_item",
),
dbc.AccordionItem(
children=[
dbc.Button(children=["Page 2"], className="accordion-item-button", key="/page-2", href="/page-2")
dbc.Button(
children=["Page 2"], className="accordion-item-button", active=False, key="/page-2", href="/page-2"
)
],
title="PAGE 2",
class_name="accordion_item",
Expand All @@ -81,6 +88,7 @@ def accordion_from_pages_as_dict():
class_name="accordion",
persistence=True,
persistence_type="session",
always_open=True,
),
html.Hr(),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading