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.

Sign up for GitHub

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

Update to use dmc 0.15.1 #924

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
+58 −275
Draft

Update to use dmc 0.15.1 #924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1a962f1
Update to use dmc 0.15.1
AnnMarieW Dec 7, 2024
7ff1b73
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 7, 2024
efbd801
Apply suggestions from code review
AnnMarieW Dec 12, 2024
7e1ff1c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 12, 2024
4a091d2
Update after review
AnnMarieW Dec 15, 2024
fd2aeff
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 15, 2024
4189fa8
added example app
AnnMarieW Dec 16, 2024
cfb1c3b
example of mantine style
AnnMarieW Dec 16, 2024
40aba6d
update after review
AnnMarieW Jan 6, 2025
c216c98
fixed tests
AnnMarieW Jan 6, 2025
e3d446c
update after review
AnnMarieW Jan 8, 2025
5b50548
Update vizro-core/src/vizro/models/_components/form/date_picker.py
AnnMarieW Jan 16, 2025
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
14 changes: 7 additions & 7 deletions vizro-core/examples/dev/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,13 +820,13 @@ def multiple_cards(data_frame: pd.DataFrame, n_rows: Optional[int] = 1) -> html.

if __name__ == "__main__":
app = Vizro().build(dashboard)
app.dash.layout.children.append(
dbc.NavLink(
["Made with ", html.Img(src=get_asset_url("logo.svg"), id="banner", alt="Vizro logo"), "vizro"],
href="https://github.com/mckinsey/vizro",
target="_blank",
className="anchor-container",
)

Copy link
Contributor

@antonymilne antonymilne Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Just to understand: why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously it was like this:

app.layout = html.Div([...])

Now it's like

app,layout= dmc.MantineProvider(html.Div([...])

So, previously, the app.layout.children was a list, and now it's a single component. If it's a common pattern to add components to the layout like in this app, then we should make sure that children is a list, like below:

app.layout = dmc.MantineProvider([html.Div[]])

banner = dbc.NavLink(
["Made with ", html.Img(src=get_asset_url("logo.svg"), id="banner", alt="Vizro logo"), "vizro"],
href="https://github.com/mckinsey/vizro",
target="_blank",
className="anchor-container",
)
app.dash.layout.children = [app.dash.layout.children, banner]
server = app.dash.server
app.run()
81 changes: 8 additions & 73 deletions vizro-core/examples/scratch_dev/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,89 +1,24 @@
"""Dev app to try things out."""

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

df = px.data.gapminder()
gapminder_data = (
df.groupby(by=["continent", "year"]).agg({"lifeExp": "mean", "pop": "sum", "gdpPercap": "mean"}).reset_index()
)
first_page = vm.Page(
title="First Page",
layout=vm.Layout(grid=[[0, 0], [1, 2], [1, 2], [1, 2]]),
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.Graph(
id="box_cont",
figure=px.box(
gapminder_data,
x="continent",
y="lifeExp",
color="continent",
labels={"lifeExp": "Life Expectancy", "continent": "Continent"},
),
),
vm.Graph(
id="line_gdp",
figure=px.line(
gapminder_data,
x="year",
y="gdpPercap",
color="continent",
labels={"year": "Year", "continent": "Continent", "gdpPercap": "GDP Per Cap"},
),
),
],
controls=[
vm.Filter(column="continent", targets=["box_cont", "line_gdp"]),
],
)
stocks = px.data.stocks(datetimes=True)

iris_data = px.data.iris()
second_page = vm.Page(
title="Second Page",
page = vm.Page(
title="Page",
components=[
vm.Graph(
id="scatter_iris",
figure=px.scatter(
iris_data,
x="sepal_width",
y="sepal_length",
color="species",
color_discrete_map={"setosa": "#00b4ff", "versicolor": "#ff9222"},
labels={"sepal_width": "Sepal Width", "sepal_length": "Sepal Length", "species": "Species"},
),
),
vm.Graph(
id="hist_iris",
figure=px.histogram(
iris_data,
x="sepal_width",
color="species",
color_discrete_map={"setosa": "#00b4ff", "versicolor": "#ff9222"},
labels={"sepal_width": "Sepal Width", "count": "Count", "species": "Species"},
),
figure=px.line(stocks, x="date", y="GOOG", title="Stocks Data"),
),
],
controls=[
vm.Parameter(
targets=["scatter_iris.color_discrete_map.virginica", "hist_iris.color_discrete_map.virginica"],
selector=vm.Dropdown(options=["#ff5267", "#3949ab"], multi=False, value="#3949ab", title="Color Virginica"),
),
vm.Parameter(
targets=["scatter_iris.opacity"],
selector=vm.Slider(min=0, max=1, value=0.8, title="Opacity"),
),
vm.Filter(column="GOOG"),
vm.Filter(column="date", selector=vm.DatePicker(title="Date Picker (Stocks - date)")),
],
)

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

if __name__ == "__main__":
Vizro().build(dashboard).run()
Vizro().build(dashboard).run()
2 changes: 1 addition & 1 deletion vizro-core/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies = [
"pandas>=2",
"plotly>=5.12.0",
"pydantic>=1.10.16", # must be synced with pre-commit mypy hook manually
"dash_mantine_components<0.13.0", # 0.13.0 is not compatible with 0.12,
"dash_mantine_components==0.15.1", # 0.13.0 is not compatible with 0.12,
Copy link
Contributor

Choose a reason for hiding this comment

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

Due to https://iscinumpy.dev/post/bound-version-constraints/, I think we should remove the upper-bound version constraint (actually the version pinning in this case).

Suggested change
"dash_mantine_components==0.15.1", # 0.13.0 is not compatible with 0.12,
"dash_mantine_components>=0.15.1",

Or maybe:

Suggested change
"dash_mantine_components==0.15.1", # 0.13.0 is not compatible with 0.12,
"dash_mantine_components>=0.15.0", # 0.13.0 is not compatible with 0.12,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With DMC I think it's best to pin an exact version. We are not yet at a major release (that's coming soon), plus there will be a breaking change when Dash 3.0 comes out that will require an update to DMC.

Whenever this gets merged, I can help with managing dependencies (at least until we get to a major release)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds great! Thank you 😃

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to do dash_mantine_components~=0.15.1 so we get updates like 0.15.2?

Copy link
Contributor

Choose a reason for hiding this comment

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

Before, I was a huge fan of ~=X.Y, ~=X.Y.Z, <=X notations, but after reading this article I changed my mind a bit. Now, I'm not to strict about the silver bullet solution for this topic so I also give a chance to the >=X.Y.Z notation.

Actually, for this case, I prefer this (>=X.Y.Z) notation because we are not sure that the dmc==0.16.0 will break the current dmc.DatePickerInput (0.15.0 ). I prefer using ~=, == or <= notation only if it's much more likely that the new version will break something. Otherwise, let it update, and if something goes wrong (which doesn't happen as ofter as I though before), we can easily pin the lower version until Vizro become adjusted to the breaking change. Guided by those thoughts, I guess we should leave dash_maintain_components>=0.15.1, but also pin the dash version to dash<3.0.0 (as we already did), because it's more likely that dash==3.0.0 will break something in Vizro .

"flask_caching>=2",
"wrapt>=1",
"black",
Expand Down
11 changes: 11 additions & 0 deletions vizro-core/src/vizro/_vizro.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import TYPE_CHECKING, TypedDict, cast

import dash
import dash_mantine_components as dmc
import plotly.io as pio
from dash.development.base_component import ComponentRegistry
from flask_caching import SimpleCache
Expand All @@ -17,6 +18,9 @@
from vizro.managers import data_manager, model_manager
from vizro.models import Dashboard, Filter

# this can be removed when Dash uses React 18 as a default (likely V3.0 https://github.com/plotly/dash/pull/3093)
dash._dash_renderer._set_react_version("18.2.0")

Comment on lines +22 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

In this comment, you said that:

Mantine V7 requires React 18, which will be the default in Dash 3.0. For now, React 18 needs to be explicitly set in the app or via an environment variable.

I'm missing something here. I see that _set_react_version("18.2.0") is suggested even in the official README.md from the dmc repo.

However, I'm a bit confused. Let's say that the following dependency is set in Vizro -> "dash_mantine_components>=0.15.1". Isn't that "react==18.2.0" will be automatically installed as dmc==0.15.1 has that constraint in its package.json file? Why should we handle any of dependency versions explicitly from the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that the React version used at runtime is controlled by the Dash renderer. The current default is React v16.14.0.

Mantine requires a minimum of React 18, which is why it needs to be set explicitly. This won't be necessary in Dash 3.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Oke 👍 It would be helpful if you can add a comment to consider removing this line when we increase the dash dependency to >=3.0.0?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is setting this react version likely to cause problems with anything else? I don't know whether some Dash components only function with React v16.14.0?

logger = logging.getLogger(__name__)

if TYPE_CHECKING:
Expand Down Expand Up @@ -49,6 +53,13 @@ def __init__(self, **kwargs):
use_pages=True,
)

# Ensure external_stylesheets is a list and append the additional stylesheet
external_stylesheets = self.dash.config.external_stylesheets
self.dash.config.external_stylesheets = (
external_stylesheets if isinstance(external_stylesheets, list) else [external_stylesheets]
)
antonymilne marked this conversation as resolved.
Show resolved Hide resolved
self.dash.config.external_stylesheets.append(dmc.styles.DATES)

# When Vizro is used as a framework, we want to include the library and framework resources.
# Dash serves resources in the order 1. external_stylesheets/scripts; 2. library resources from the
# ComponentRegistry; 3. resources added by append_css/scripts.
Expand Down
49 changes: 8 additions & 41 deletions vizro-core/src/vizro/models/_components/form/date_picker.py
View file
Open in desktop
petar-qb marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
from typing import Literal, Optional, Union

import dash_mantine_components as dmc
from dash import ClientsideFunction, Input, Output, State, clientside_callback, dcc, html
from dash import html

try:
from pydantic.v1 import Field, PrivateAttr, validator
except ImportError: # pragma: no cov
from pydantic import Field, PrivateAttr, validator


import datetime
from datetime import date

import dash_bootstrap_components as dbc
Expand Down Expand Up @@ -42,6 +41,7 @@ class DatePicker(VizroBaseModel):
max: Optional[date] = Field(None, description="End date for date picker.")
value: Optional[Union[list[date], date]] = Field(None, description="Default date for date picker")
title: str = Field("", description="Title to be displayed.")

range: bool = Field(True, description="Boolean flag for displaying range picker.")
actions: list[Action] = []

Expand All @@ -55,57 +55,24 @@ class DatePicker(VizroBaseModel):

def build(self):
init_value = self.value or ([self.min, self.max] if self.range else self.min) # type: ignore[list-item]
date_range_picker_kwargs = {"allowSingleDateInRange": True} if self.range else {}

output = [
Output(self.id, "value"),
Output(f"{self.id}_input_store", "data"),
]
inputs = [
Input(self.id, "value"),
State(f"{self.id}_input_store", "data"),
]

clientside_callback(
ClientsideFunction(namespace="date_picker", function_name="update_date_picker_values"),
output=output,
inputs=inputs,
)
# clientside callback is required as a workaround when the date-picker is overflowing its parent container
# if there is not enough space. Caused by another workaround for this issue:
# https://github.com/snehilvj/dash-mantine-components/issues/219
clientside_callback(
ClientsideFunction(namespace="date_picker", function_name="update_date_picker_position"),
output=Output(self.id, "dropdownPosition"),
inputs=Input(self.id, "n_clicks"),
)
petar-qb marked this conversation as resolved.
Show resolved Hide resolved

date_picker_class = dmc.DateRangePicker if self.range else dmc.DatePicker

# dropdownPosition must be set to bottom-start as a workaround for issue:
# https://github.com/snehilvj/dash-mantine-components/issues/219
# clearable must be set to False as a workaround for issue:
# https://github.com/snehilvj/dash-mantine-components/issues/212
# maxDate must be increased by one day, and later on disabledDates must be set as maxDate + 1 day
# as a workaround for issue: https://github.com/snehilvj/dash-mantine-components/issues/230
date_picker = date_picker_class(
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved
date_picker = dmc.DatePickerInput(
petar-qb marked this conversation as resolved.
Show resolved Hide resolved
id=self.id,
minDate=self.min,
value=init_value,
maxDate=self.max + datetime.timedelta(days=1) if self.max else None,
maxDate=self.max,
persistence=True,
persistence_type="session",
dropdownPosition="bottom-start",
clearable=False,
disabledDates=self.max + datetime.timedelta(days=1) if self.max else None,
type="range" if self.range else "default",
allowSingleDateInRange= True,
className="datepicker",
**date_range_picker_kwargs,
# removes the default red color for weekend days
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually want this or was it just to demonstrate something? We don't normally do inline styles like this. @huong-li-nguyen @petar-qb

Copy link
Contributor

Choose a reason for hiding this comment

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

AM added this to demonstrate how to update some of the styles. I would ignore all CSS changes in this branch and wait for @nadijagraca PR today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be better to do dash_mantine_components~=0.15.1 so we get updates like 0.15.2?

Yes, I think that would be OK

Is setting this react version likely to cause problems with anything else? I don't know whether some Dash components only function with React v16.14.0?

Plotly has been testing with React 18 for over a year. There hasn't been any issues that I know of. I was just reminded that Dash 3.0 will be using React 18.3.1, and in that version there will be a deprecation warning in the browser console for any components that use the defaultProps function. Both DMC and DBC and Dash AG Grid are on a path to fix this. I'm not sure about any other community libraries. The Dash component libraries, dcc, html and DataTable have been updated. Pinning it to React 18.2.1 would be good until the other libraries are ready.

Do we actually want this or was it just to demonstrate something? We don't normally do inline styles like this

It would be better if the styles are applied in the theme prop in the MantineProvider, then it would also apply to any custom DMC component that was added.

I'd be happy to join a call to discuss CSS if you like. Mantine has a different way to style apps and components than Bootstrap, and I could help highlight the differences and leverage the Mantine system to make it easier to maintain going forward. In the meantime, here are a few links that might help:

  • Mantine API Overiew - great starting point for understanding styling in DMC>=0.14
  • Forum Post on DMC theme builder - When I started playing with this app I had an "ah-ha" moment on the power of the Mantine theme object which sets global style defaults.
  • Styles API - how to set component level styles either globally or on individual components.
  • The whole "Theming" category in the docs has been updated recently and has lots of great info.

Copy link
Contributor

@huong-li-nguyen huong-li-nguyen Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@nadijagraca has opened a PR here: AnnMarieW#1
From my side, it has all the visual changes we need at the moment.

@nadijagraca - could you check the links above and see whether we can refactor our CSS some more? :) For example, it seems that we could set a border-radius of 0 for all dmc components on the Mantine theme object etc.
@AnnMarieW - feel free to take a look at that PR as well 👍

I also don't want to drag this PR for too long, so from my side, we can merge and refactor the CSS in another PR.

styles={"day": {"color": "var(--mantine-color-text"}},
)

return html.Div(
children=[
dbc.Label(children=self.title, html_for=self.id) if self.title else None,
date_picker,
dcc.Store(id=f"{self.id}_input_store", storage_type="session", data=init_value),
],
)
8 changes: 7 additions & 1 deletion vizro-core/src/vizro/models/_dashboard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import dash
import dash_bootstrap_components as dbc
import dash_mantine_components as dmc
import plotly.io as pio
from dash import (
ClientsideFunction,
Expand Down Expand Up @@ -156,7 +157,7 @@ def build(self):
State("collapsable-left-side", "is_open"),
)

return html.Div(
layout = html.Div(
id="dashboard-container",
children=[
html.Div(id="vizro_version", children=vizro.__version__, hidden=True),
Expand All @@ -171,6 +172,11 @@ def build(self):
dash.page_container,
],
)
return dmc.MantineProvider(
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually need this? I read through the dmc docs and played around and as far as I can tell the only thing that dmc.MantineProvider does is set <head data-mantine-color-scheme=...>. You already added a line to set this variable in update_dashboard_theme, which always runs when the page is first loaded. Hence unless dmc.MantineProvider does something in addition to setting <head data-mantine-color-scheme=...> this would seem redundant because it just gets overidden straight away by the update_dashboard_theme callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the MantineProvider does more than just set the theme. It's required as of DMC 14.0 (Mantine 7) No Mantine component will render if the MantineProvider is not included in the app. More info in Mantine API Overiew

Copy link
Contributor

@huong-li-nguyen huong-li-nguyen Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Apart from that, it seems like we need it anyway. I've added the font-family to @nadijagraca CSS PR, as otherwise, you would have to set it in several classes. This seemed to be the cleanest way to apply the global font to all dmc components that will be potentially added.

dmc.MantineProvider(
            layout,
            # Applies to all Mantine components
            theme={"fontFamily": "Inter, sans-serif, Arial, serif"},
        )

layout,
# Use the `theme` to style all Mantine components with a Vizro theme. For more info see https://www.dash-mantine-components.com/components/mantineprovider
theme = {"primaryColor": "gray"}
)

def _validate_logos(self):
logo_img = self._infer_image(filename="logo")
Expand Down
109 changes: 0 additions & 109 deletions vizro-core/src/vizro/static/css/datepicker.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,109 +0,0 @@
.datepicker .mantine-Input-wrapper {
font-family: unset;
height: 2rem;
}

.datepicker .mantine-DateRangePicker-input,
.datepicker .mantine-DatePicker-input {
background-color: var(--field-enabled);
border: none;
border-radius: 0;
box-shadow: var(--elevation-0);
color: var(--text-secondary);
font-size: 0.875rem;
height: 2rem;
line-height: 1rem;
min-height: 2rem;
padding: 0 0.5rem;
}

.datepicker .mantine-DateRangePicker-input:hover,
.datepicker .mantine-DatePicker-input:hover {
color: var(--text-primary);
}

.datepicker .mantine-DateRangePicker-dropdown,
.datepicker .mantine-DatePicker-dropdown {
background: var(--field-enabled);
border: none;
border-radius: 0;
box-shadow: var(--elevation-1);
padding: 1rem 11px; /* 11px otherwise not aligned with controls */
}

.datepicker .mantine-UnstyledButton-root {
border-radius: 0;
color: var(--text-secondary);
font-family: unset;
font-weight: 400;
}

.datepicker .mantine-UnstyledButton-root:hover {
background: var(--stateOverlays-hover);
color: var(--text-primary);
}

.datepicker .mantine-DateRangePicker-weekday,
.datepicker .mantine-DatePicker-weekday {
color: var(--text-secondary);
font-family: unset;
padding: 0.5rem;
}

.datepicker .mantine-DateRangePicker-cell,
.datepicker .mantine-DatePicker-cell {
border: none;
}

.datepicker .mantine-DateRangePicker-day,
.datepicker .mantine-DatePicker-day {
background: var(--field-enabled);
border-radius: 0;
color: var(--text-secondary);
font-family: unset;
}

.datepicker .mantine-DateRangePicker-day[data-outside],
.datepicker .mantine-DateRangePicker-day:disabled,
.datepicker .mantine-DatePicker-day[data-outside],
.datepicker .mantine-DatePicker-day:disabled,
.datepicker .mantine-UnstyledButton-root:disabled {
color: var(--text-disabled);
}

.datepicker .mantine-DateRangePicker-day:hover,
.datepicker .mantine-DateRangePicker-day:focus-visible,
.datepicker .mantine-DatePicker-day:hover,
.datepicker .mantine-DatePicker-day:focus-visible {
background: var(--stateOverlays-hover);
border: none;
color: var(--text-primary);
outline: none;
text-decoration: none;
}

.datepicker .mantine-DateRangePicker-day[data-in-range] {
background: var(--stateOverlays-selected);
color: var(--text-primary);
}

.datepicker .mantine-DateRangePicker-day[data-selected],
.datepicker .mantine-DateRangePicker-yearPickerControlActive,
.datepicker .mantine-DateRangePicker-monthPickerControlActive,
.datepicker .mantine-DatePicker-day[data-selected],
.datepicker .mantine-DatePicker-yearPickerControlActive,
.datepicker .mantine-DatePicker-monthPickerControlActive {
background: var(--stateOverlays-selected-inverted);
color: var(--text-primary-inverted);
text-decoration: underline;
}

.datepicker
.mantine-DateRangePicker-calendarHeader
.mantine-UnstyledButton-root:hover,
.datepicker
.mantine-DatePicker-calendarHeader
.mantine-UnstyledButton-root:hover {
background: transparent;
color: var(--text-primary);
}
Loading