-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add multi select filter #174
Add multi select filter #174
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nfx : The filter needs some rethinking as in Lakeview this is another widget (not part of the the table). Filters can actually operate on multiple widgets if they use the same dataset, but we create a dataset and widget per query, so there would be a one-to-one relationship in our case.
widget = Widget(name=f"{self._widget_metadata.id}_filter", queries=[named_query], spec=spec) | ||
return widget | ||
|
||
def _get_named_query(self, fields: list[Field], *, name: str = "main_query", disaggregated: bool = True) -> NamedQuery: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not really do much anymore besides passing parameters
layout = Layout(widget=tile.widget, position=tile.position) | ||
layouts.append(layout) | ||
if isinstance(tile, QueryTile) and tile.filter is not None: | ||
layout = Layout(widget=tile.filter, position=tile.position) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to figure out what to do with the filter position
@@ -377,6 +410,9 @@ def _get_layouts(widgets_metadata: list[WidgetMetadata]) -> list[Layout]: | |||
placed_tile = tile.place_after(position) | |||
layout = Layout(widget=placed_tile.widget, position=placed_tile.position) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nfx : Maybe we want to convert the widget
on a tile to widgets
. Might be useful in future cases, or at least for multiple filters (on different columns)
@@ -81,16 +83,18 @@ def __init__( | |||
width: int = 0, | |||
height: int = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WidgetMetdata
is not becoming accurate anymore. It would be TileMetadata
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm to unblock further work
68f524d
to
d4946b7
Compare
Preferences goes to #181 |
Resolves #169
Stacked on #150