From 4584c9b3974561439584fa74f9d03ed9afc5f651 Mon Sep 17 00:00:00 2001
From: Paul Rosenzweig <paulrosenzweig@users.noreply.github.com>
Date: Fri, 26 Jul 2019 21:10:23 -0400
Subject: [PATCH] In settings, show selected option even when the default is
selected (#10411)
---
.../components/widgets/SettingSelect.jsx | 35 +++++++++------
.../src/metabase/admin/settings/selectors.js | 7 +--
frontend/src/metabase/components/Select.jsx | 6 ++-
.../metabase/components/Select.unit.spec.js | 45 +++++++++++++++++++
4 files changed, 72 insertions(+), 21 deletions(-)
create mode 100644 frontend/test/metabase/components/Select.unit.spec.js
diff --git a/frontend/src/metabase/admin/settings/components/widgets/SettingSelect.jsx b/frontend/src/metabase/admin/settings/components/widgets/SettingSelect.jsx
index ec49282e6a9..72c493af863 100644
--- a/frontend/src/metabase/admin/settings/components/widgets/SettingSelect.jsx
+++ b/frontend/src/metabase/admin/settings/components/widgets/SettingSelect.jsx
@@ -1,22 +1,29 @@
import React from "react";
-import Select from "metabase/components/Select.jsx";
-import _ from "underscore";
+import Select, { Option } from "metabase/components/Select.jsx";
-const SettingSelect = ({ setting, onChange, disabled }) => (
+const SettingSelect = ({
+ setting: { placeholder, value, options, defaultValue },
+ onChange,
+ disabled,
+}) => (
<Select
className="full-width"
- placeholder={setting.placeholder}
- value={
- _.findWhere(setting.options, { value: setting.value }) || setting.value
- }
- options={setting.options}
- onChange={onChange}
- optionNameFn={option => (typeof option === "object" ? option.name : option)}
- optionValueFn={option =>
- typeof option === "object" ? option.value : option
- }
- />
+ placeholder={placeholder}
+ value={value}
+ defaultValue={defaultValue}
+ onChange={e => onChange(e.target.value)}
+ >
+ {options.map(option => {
+ const name = typeof option === "object" ? option.name : option;
+ const value = typeof option === "object" ? option.value : option;
+ return (
+ <Option key={value} name={name} value={value}>
+ {name}
+ </Option>
+ );
+ })}
+ </Select>
);
export default SettingSelect;
diff --git a/frontend/src/metabase/admin/settings/selectors.js b/frontend/src/metabase/admin/settings/selectors.js
index c324ec85ef0..18f5be6d471 100644
--- a/frontend/src/metabase/admin/settings/selectors.js
+++ b/frontend/src/metabase/admin/settings/selectors.js
@@ -51,7 +51,6 @@ const SECTIONS = [
{ name: t`Database Default`, value: "" },
...MetabaseSettings.get("timezones"),
],
- placeholder: t`Select a timezone`,
note: t`Not all databases support timezones, in which case this setting won't take effect.`,
allowValueCollection: true,
},
@@ -62,7 +61,7 @@ const SECTIONS = [
options: (MetabaseSettings.get("available_locales") || []).map(
([value, name]) => ({ name, value }),
),
- placeholder: t`Select a language`,
+ defaultValue: "en",
getHidden: () => MetabaseSettings.get("available_locales").length < 2,
},
{
@@ -82,9 +81,7 @@ const SECTIONS = [
},
{ value: "none", name: t`Disabled` },
],
- // this needs to be here because 'advanced' is the default value, so if you select 'advanced' the
- // widget will always show the placeholder instead of the 'name' defined above :(
- placeholder: t`Enabled`,
+ defaultValue: "advanced",
},
{
key: "enable-nested-queries",
diff --git a/frontend/src/metabase/components/Select.jsx b/frontend/src/metabase/components/Select.jsx
index 3c14be8354d..8c25a4cea80 100644
--- a/frontend/src/metabase/components/Select.jsx
+++ b/frontend/src/metabase/components/Select.jsx
@@ -36,6 +36,7 @@ class BrowserSelect extends Component {
children: PropTypes.array.isRequired,
onChange: PropTypes.func.isRequired,
value: PropTypes.any,
+ defaultValue: PropTypes.any,
searchProp: PropTypes.string,
searchCaseInsensitive: PropTypes.bool,
@@ -66,14 +67,15 @@ class BrowserSelect extends Component {
};
isSelected(otherValue) {
- const { value, multiple } = this.props;
+ const { value, multiple, defaultValue } = this.props;
if (multiple) {
return _.any(value, v => v === otherValue);
} else {
return (
value === otherValue ||
((value == null || value === "") &&
- (otherValue == null || otherValue === ""))
+ (otherValue == null || otherValue === "")) ||
+ (value == null && otherValue === defaultValue)
);
}
}
diff --git a/frontend/test/metabase/components/Select.unit.spec.js b/frontend/test/metabase/components/Select.unit.spec.js
new file mode 100644
index 00000000000..517e4aeb705
--- /dev/null
+++ b/frontend/test/metabase/components/Select.unit.spec.js
@@ -0,0 +1,45 @@
+import React from "react";
+import { render, cleanup } from "@testing-library/react";
+
+import Select, { Option } from "metabase/components/Select";
+
+describe("Select", () => {
+ afterEach(cleanup);
+
+ it("should render selected option", () => {
+ const { getByText, queryByText } = render(
+ <Select value="b">
+ <Option value="a">option a</Option>
+ <Option value="b">option b</Option>
+ </Select>,
+ );
+
+ expect(queryByText("option a")).toBe(null);
+ getByText("option b");
+ });
+
+ it("should render the defaultValue if none is selected", () => {
+ const { getByText, queryByText } = render(
+ <Select defaultValue="b">
+ <Option value="a">option a</Option>
+ <Option value="b">option b</Option>
+ </Select>,
+ );
+
+ expect(queryByText("option a")).toBe(null);
+ getByText("option b");
+ });
+
+ it("should render a placeholder if none is selected", () => {
+ const { getByText, queryByText } = render(
+ <Select placeholder="choose an option">
+ <Option value="a">option a</Option>
+ <Option value="b">option b</Option>
+ </Select>,
+ );
+
+ expect(queryByText("option a")).toBe(null);
+ expect(queryByText("option b")).toBe(null);
+ getByText("choose an option");
+ });
+});
--
GitLab