From 3ec2c8a83cee61a8150a91002614c9fafd11aa20 Mon Sep 17 00:00:00 2001
From: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Date: Wed, 11 Aug 2021 07:30:06 -0700
Subject: [PATCH] Fix column settings button if column settings side bar is
already open (#17355)
---
.../src/metabase/query_builder/reducers.js | 1 +
.../settings/ChartNestedSettingColumns.jsx | 21 ++++++++++++
.../settings/ChartSettingNestedSettings.jsx | 32 ++++++-------------
.../scenarios/question/settings.cy.spec.js | 8 ++++-
4 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/frontend/src/metabase/query_builder/reducers.js b/frontend/src/metabase/query_builder/reducers.js
index 59b229c2680..7851e40f610 100644
--- a/frontend/src/metabase/query_builder/reducers.js
+++ b/frontend/src/metabase/query_builder/reducers.js
@@ -169,6 +169,7 @@ export const uiControls = handleActions(
[SHOW_CHART_SETTINGS]: {
next: (state, { payload }) => ({
...state,
+ ...UI_CONTROLS_SIDEBAR_DEFAULTS,
isShowingChartSettingsSidebar: true,
initialChartSetting: payload,
}),
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartNestedSettingColumns.jsx b/frontend/src/metabase/visualizations/components/settings/ChartNestedSettingColumns.jsx
index e40557cd48c..494cc71bd52 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartNestedSettingColumns.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartNestedSettingColumns.jsx
@@ -56,6 +56,27 @@ class ColumnWidgets extends React.Component {
}
}
+ componentDidUpdate(prevProps) {
+ const {
+ setSidebarPropsOverride,
+ object,
+ onEndShowWidget,
+ currentSectionHasColumnSettings,
+ } = this.props;
+
+ if (
+ displayNameForColumn(object) !== displayNameForColumn(prevProps.object) ||
+ onEndShowWidget !== prevProps.onEndShowWidget
+ ) {
+ if (setSidebarPropsOverride && !currentSectionHasColumnSettings) {
+ setSidebarPropsOverride({
+ title: displayNameForColumn(object),
+ onBack: onEndShowWidget,
+ });
+ }
+ }
+ }
+
componentWillUnmount() {
const { setSidebarPropsOverride } = this.props;
if (setSidebarPropsOverride) {
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingNestedSettings.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingNestedSettings.jsx
index 2a6dcc221cf..e59f7aa3969 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingNestedSettings.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingNestedSettings.jsx
@@ -64,29 +64,18 @@ const chartSettingNestedSettings = ({
constructor(props: Props) {
super(props);
- this.state = {
- editingObjectKey:
- props.initialKey ||
- (props.objects.length === 1 ? getObjectKey(props.objects[0]) : null),
- };
}
- UNSAFE_componentWillReceiveProps(nextProps: Props) {
- // reset editingObjectKey if there's only one object
- if (
- nextProps.objects.length === 1 &&
- this.state.editingObjectKey !== getObjectKey(nextProps.objects[0])
- ) {
- this.setState({
- editingObjectKey: getObjectKey(nextProps.objects[0]),
- });
- }
- }
+ getEditingObjectKey = () => {
+ return (
+ this.props.initialKey ||
+ (this.props.objects.length === 1
+ ? getObjectKey(this.props.objects[0])
+ : null)
+ );
+ };
handleChangeEditingObject = (editingObject: ?NestedObject) => {
- this.setState({
- editingObjectKey: editingObject ? getObjectKey(editingObject) : null,
- });
// special prop to notify ChartSettings it should unswap replaced widget
if (!editingObject && this.props.onEndShowWidget) {
this.props.onEndShowWidget();
@@ -94,7 +83,7 @@ const chartSettingNestedSettings = ({
};
handleChangeSettingsForEditingObject = (newSettings: Settings) => {
- const { editingObjectKey } = this.state;
+ const editingObjectKey = this.getEditingObjectKey();
if (editingObjectKey) {
this.handleChangeSettingsForObjectKey(editingObjectKey, newSettings);
}
@@ -126,8 +115,7 @@ const chartSettingNestedSettings = ({
render() {
const { series, objects, extra } = this.props;
- const { editingObjectKey } = this.state;
-
+ const editingObjectKey = this.getEditingObjectKey();
if (editingObjectKey) {
const editingObject = _.find(
objects,
diff --git a/frontend/test/metabase/scenarios/question/settings.cy.spec.js b/frontend/test/metabase/scenarios/question/settings.cy.spec.js
index 18892a1e3d4..23ffdd6cdca 100644
--- a/frontend/test/metabase/scenarios/question/settings.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/settings.cy.spec.js
@@ -164,7 +164,13 @@ describe("scenarios > question > settings", () => {
popover().within(() => cy.icon("gear").click()); // open subtotal column settings
cy.findByText("Table options").should("not.exist"); // no longer displaying the top level settings
- cy.findByText("Column title"); // shows subtotal column settings
+ cy.findByText("Separator style"); // shows subtotal column settings
+
+ cy.get(".TableInteractive")
+ .findByText("Created At")
+ .click(); // open created_at column header actions
+ popover().within(() => cy.icon("gear").click()); // open created_at column settings
+ cy.findByText("Date style"); // shows created_at column settings
});
});
--
GitLab