Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix column style type reactivity to View changes #2535

Merged
merged 2 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -120,7 +120,7 @@ impl ColumnSettingsSidebar {
self.save_enabled = changed && valid;
}

fn initialize(ctx: &yew::prelude::Context<Self>) -> Self {
fn initialize(&mut self, ctx: &yew::prelude::Context<Self>) {
let column_name = ctx
.props()
.selected_column
Expand All @@ -134,11 +134,7 @@ impl ColumnSettingsSidebar {
let initial_expr_value = Rc::new(initial_expr_value);
let initial_header_value =
(*initial_expr_value != column_name).then_some(column_name.clone());
let maybe_ty = ctx
.props()
.session
.metadata()
.get_column_view_type(&column_name);
let maybe_ty = ctx.props().selected_column.view_type(ctx.props().session());

// NOTE: This is going to be refactored soon.
let tabs = {
Expand Down Expand Up @@ -177,7 +173,7 @@ impl ColumnSettingsSidebar {
tabs
};

Self {
*self = Self {
column_name,
expr_value: initial_expr_value.clone(),
initial_expr_value,
Expand All @@ -186,7 +182,8 @@ impl ColumnSettingsSidebar {
maybe_ty,
tabs,
header_valid: true,
..Default::default()
session_sub: self.session_sub.take(),
..*self
}
}
}
Expand All @@ -196,7 +193,6 @@ impl Component for ColumnSettingsSidebar {
type Properties = ColumnSettingsProps;

fn create(ctx: &yew::prelude::Context<Self>) -> Self {
let mut this = Self::initialize(ctx);
let session_cb = ctx
.link()
.callback(|_| ColumnSettingsMsg::SessionUpdated(()));
Expand All @@ -205,14 +201,19 @@ impl Component for ColumnSettingsSidebar {
.renderer
.session_changed
.add_listener(session_cb);
this.session_sub = Some(session_sub);

let mut this = Self {
session_sub: Some(session_sub),
..Default::default()
};
this.initialize(ctx);
this
}

fn changed(&mut self, ctx: &yew::prelude::Context<Self>, old_props: &Self::Properties) -> bool {
if ctx.props() != old_props {
let selected_tab = self.selected_tab;
*self = Self::initialize(ctx);
self.initialize(ctx);
self.selected_tab = selected_tab;
self.selected_tab_idx = self
.tabs
Expand Down Expand Up @@ -300,7 +301,7 @@ impl Component for ColumnSettingsSidebar {
true
},
ColumnSettingsMsg::SessionUpdated(()) => {
*self = Self::initialize(ctx);
self.initialize(ctx);
true
},
}
Expand Down
5 changes: 5 additions & 0 deletions rust/perspective-viewer/src/rust/components/viewer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ impl ColumnLocator {
pub fn is_expr(&self) -> bool {
matches!(self, ColumnLocator::Expr(_))
}

pub fn view_type(&self, session: &Session) -> Option<Type> {
let name = self.name().cloned().unwrap_or_default();
session.metadata().get_column_view_type(name.as_str())
}
}

#[derive(Properties)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type Action =
| "deactivate"
| "reorder"
| "aggregate"
| "deaggregate"
| "groupby"
| "splitby"
| "orderby"
Expand Down Expand Up @@ -149,6 +150,23 @@ const TEST_SPEC: Record<string, TestSpec> = {
},
},
},
Deaggregate: {
initial_state: {
open: true,
active: true,
},
actions: ["deaggregate"],
outputs: {
table_col: {
unchanged: true,
type: "string",
},
expr_col: {
unchanged: true,
type: "string",
},
},
},
"Groupby - Active": {
initial_state: {
open: true,
Expand Down Expand Up @@ -263,7 +281,11 @@ async function checkOutput(
) {
const POSSIBLE_TABS = ["Style", "Attributes"];

const check_tabs = async (selectedTab, tabs, unexpected_tabs) => {
const check_tabs = async (
selectedTab: string,
tabs: string[],
unexpected_tabs: string[]
) => {
await view.columnSettingsSidebar.container
.locator(".tab.selected")
.getByText(selectedTab)
Expand Down Expand Up @@ -333,7 +355,7 @@ test.describe("Column Settings State on Interaction", () => {
"orderby",
"where",
],
text: ["reorder", "open", "deactivate"],
text: ["reorder", "open", "deactivate", "deaggregate"],
dnd: ["open", "deactivate", "groupby", "splitby"],
};

Expand Down Expand Up @@ -366,6 +388,14 @@ test.describe("Column Settings State on Interaction", () => {
await selector.activeBtn.click();
break;
}
case "deaggregate": {
await pageView.settingsPanel.groupby("Row ID");
await pageView.settingsPanel.removeViewParameter(
"groupby",
"Row ID"
);
break;
}
default: {
test.fail(true, "Unreachable");
}
Expand Down Expand Up @@ -488,6 +518,21 @@ test.describe("Column Settings State on Interaction", () => {
);
break;
}
case "deaggregate": {
let aggregator =
pageView.settingsPanel.inactiveColumns.columnSelector.first();
await aggregator.dragTo(
pageView.settingsPanel.groupbyInput
);
aggregator = page
.locator("#group_by .pivot-column")
.first();
await aggregator.dragTo(
pageView.settingsPanel.activeColumns.container
.locator(".column-selector-column")
.first()
);
}
}
await checkOutput(
pageView,
Expand Down
Binary file modified tools/perspective-test/results.tar.gz
Binary file not shown.
26 changes: 26 additions & 0 deletions tools/perspective-test/src/js/models/settings_panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,32 @@ export class SettingsPanel {
.waitFor();
await locator.press("Enter");
}

async removeViewParameter(type: ViewParameter, name: string) {
let locator: Locator;
switch (type) {
case "groupby":
locator = this.container.locator("#group_by .pivot-column");
break;
case "orderby":
locator = this.container.locator("#sort .pivot-column");
break;
case "splitby":
locator = this.container.locator("#split_by .pivot-column");
break;
case "where":
locator = this.container.locator("#filter .pivot-column");
break;
default:
throw "Invalid type passed!";
}
await locator
.filter({ hasText: name })
.first()
.locator(".row_close")
.click();
}

/**
* Selects a plugin by it's display name, i.e. the innerText of the .plugin-select-item
* @param name
Expand Down
Loading