From 51c466587c19ecb3fa35177b64f78c735088f100 Mon Sep 17 00:00:00 2001
From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
Date: Thu, 11 May 2023 16:58:20 +0200
Subject: [PATCH] Refactor `ListItem` component (#30650)
* Remove unused props
* Remove `index` prop
* Do not pass unneeded prop that's not even defined
* Remove seven year old TODO
* Render `ListItem` with the `li` HTML element
---
.../metabase/components/ListItem/ListItem.jsx | 52 +++++++++----------
.../components/ListItem/ListItem.unit.spec.js | 7 +--
.../reference/databases/DatabaseList.jsx | 19 +++----
.../reference/databases/TableList.jsx | 19 +++----
.../reference/databases/TableQuestions.jsx | 23 ++++----
.../metabase/reference/metrics/MetricList.jsx | 19 +++----
.../reference/metrics/MetricQuestions.jsx | 23 ++++----
.../reference/segments/SegmentList.jsx | 19 +++----
.../reference/segments/SegmentQuestions.jsx | 23 ++++----
9 files changed, 89 insertions(+), 115 deletions(-)
diff --git a/frontend/src/metabase/components/ListItem/ListItem.jsx b/frontend/src/metabase/components/ListItem/ListItem.jsx
index cd3cba72f49..11d370fbc13 100644
--- a/frontend/src/metabase/components/ListItem/ListItem.jsx
+++ b/frontend/src/metabase/components/ListItem/ListItem.jsx
@@ -8,44 +8,42 @@ import Card from "metabase/components/Card";
import S from "metabase/components/List/List.css";
import Icon from "metabase/components/Icon";
-//TODO: extend this to support functionality required for questions
-const ListItem = ({ index, name, description, placeholder, url, icon }) => (
- <Link to={url} className="text-brand-hover">
- <Card hoverable className="mb2 p3 bg-white rounded bordered">
- <div className={cx(S.item)}>
- <div className={S.itemIcons}>
- {icon && <Icon className={S.chartIcon} name={icon} size={16} />}
- </div>
- <div className={S.itemBody}>
- <div className={S.itemTitle}>
- <Ellipsified
- className={S.itemName}
- tooltip={name}
- tooltipMaxWidth="100%"
- >
- <h3>{name}</h3>
- </Ellipsified>
+const ListItem = ({ name, description, placeholder, url, icon }) => (
+ <li className="relative">
+ <Link to={url} className="text-brand-hover">
+ <Card hoverable className="mb2 p3 bg-white rounded bordered">
+ <div className={cx(S.item)}>
+ <div className={S.itemIcons}>
+ {icon && <Icon className={S.chartIcon} name={icon} size={16} />}
</div>
- {(description || placeholder) && (
- <div className={cx(S.itemSubtitle)}>
- {description || placeholder}
+ <div className={S.itemBody}>
+ <div className={S.itemTitle}>
+ <Ellipsified
+ className={S.itemName}
+ tooltip={name}
+ tooltipMaxWidth="100%"
+ >
+ <h3>{name}</h3>
+ </Ellipsified>
</div>
- )}
+ {(description || placeholder) && (
+ <div className={cx(S.itemSubtitle)}>
+ {description || placeholder}
+ </div>
+ )}
+ </div>
</div>
- </div>
- </Card>
- </Link>
+ </Card>
+ </Link>
+ </li>
);
ListItem.propTypes = {
name: PropTypes.string.isRequired,
- index: PropTypes.number.isRequired,
url: PropTypes.string,
description: PropTypes.string,
placeholder: PropTypes.string,
icon: PropTypes.string,
- isEditing: PropTypes.bool,
- field: PropTypes.object,
};
export default React.memo(ListItem);
diff --git a/frontend/src/metabase/components/ListItem/ListItem.unit.spec.js b/frontend/src/metabase/components/ListItem/ListItem.unit.spec.js
index 4d01a124e56..6d272e1ea80 100644
--- a/frontend/src/metabase/components/ListItem/ListItem.unit.spec.js
+++ b/frontend/src/metabase/components/ListItem/ListItem.unit.spec.js
@@ -6,12 +6,9 @@ import ListItem from "./ListItem";
const ITEM_NAME = "Table Foo";
const ITEM_DESCRIPTION = "Nice table description.";
-function setup({ name, index = 0, ...opts }) {
+function setup({ name, ...opts }) {
return renderWithProviders(
- <Route
- path="/"
- component={() => <ListItem name={name} index={index} {...opts} />}
- />,
+ <Route path="/" component={() => <ListItem name={name} {...opts} />} />,
{ withRouter: true },
);
}
diff --git a/frontend/src/metabase/reference/databases/DatabaseList.jsx b/frontend/src/metabase/reference/databases/DatabaseList.jsx
index 7968f61fce4..1c5f88fa3c0 100644
--- a/frontend/src/metabase/reference/databases/DatabaseList.jsx
+++ b/frontend/src/metabase/reference/databases/DatabaseList.jsx
@@ -59,17 +59,14 @@ class DatabaseList extends Component {
Object.keys(entities).length > 0 ? (
<div className="wrapper">
<List>
- {databases.map((database, index) => (
- <li className="relative" key={database.id}>
- <ListItem
- id={database.id}
- index={index}
- name={database.name}
- description={database.description}
- url={`/reference/databases/${database.id}`}
- icon="database"
- />
- </li>
+ {databases.map(database => (
+ <ListItem
+ key={database.id}
+ name={database.name}
+ description={database.description}
+ url={`/reference/databases/${database.id}`}
+ icon="database"
+ />
))}
</List>
</div>
diff --git a/frontend/src/metabase/reference/databases/TableList.jsx b/frontend/src/metabase/reference/databases/TableList.jsx
index cde47891136..4f74a79f8ad 100644
--- a/frontend/src/metabase/reference/databases/TableList.jsx
+++ b/frontend/src/metabase/reference/databases/TableList.jsx
@@ -41,17 +41,14 @@ const mapDispatchToProps = {
...metadataActions,
};
-const createListItem = (entity, index) => (
- <li className="relative" key={entity.id}>
- <ListItem
- id={entity.id}
- index={index}
- name={entity.display_name || entity.name}
- description={entity.description}
- url={`/reference/databases/${entity.db_id}/tables/${entity.id}`}
- icon="table2"
- />
- </li>
+const createListItem = entity => (
+ <ListItem
+ key={entity.id}
+ name={entity.display_name || entity.name}
+ description={entity.description}
+ url={`/reference/databases/${entity.db_id}/tables/${entity.id}`}
+ icon="table2"
+ />
);
const createSchemaSeparator = table => (
diff --git a/frontend/src/metabase/reference/databases/TableQuestions.jsx b/frontend/src/metabase/reference/databases/TableQuestions.jsx
index 55904542e87..13ab605b1dd 100644
--- a/frontend/src/metabase/reference/databases/TableQuestions.jsx
+++ b/frontend/src/metabase/reference/databases/TableQuestions.jsx
@@ -78,22 +78,19 @@ class TableQuestions extends Component {
<div className="wrapper wrapper--trim">
<List>
{Object.values(entities).map(
- (entity, index) =>
+ entity =>
entity &&
entity.id &&
entity.name && (
- <li className="relative" key={entity.id}>
- <ListItem
- id={entity.id}
- index={index}
- name={entity.display_name || entity.name}
- description={t`Created ${moment(
- entity.created_at,
- ).fromNow()} by ${entity.creator.common_name}`}
- url={Urls.question(entity)}
- icon={visualizations.get(entity.display).iconName}
- />
- </li>
+ <ListItem
+ key={entity.id}
+ name={entity.display_name || entity.name}
+ description={t`Created ${moment(
+ entity.created_at,
+ ).fromNow()} by ${entity.creator.common_name}`}
+ url={Urls.question(entity)}
+ icon={visualizations.get(entity.display).iconName}
+ />
),
)}
</List>
diff --git a/frontend/src/metabase/reference/metrics/MetricList.jsx b/frontend/src/metabase/reference/metrics/MetricList.jsx
index 23e8dfb388f..9c7f5008019 100644
--- a/frontend/src/metabase/reference/metrics/MetricList.jsx
+++ b/frontend/src/metabase/reference/metrics/MetricList.jsx
@@ -63,20 +63,17 @@ class MetricList extends Component {
<div className="wrapper wrapper--trim">
<List>
{Object.values(entities).map(
- (entity, index) =>
+ entity =>
entity &&
entity.id &&
entity.name && (
- <li className="relative" key={entity.id}>
- <ListItem
- id={entity.id}
- index={index}
- name={entity.display_name || entity.name}
- description={entity.description}
- url={`/reference/metrics/${entity.id}`}
- icon="ruler"
- />
- </li>
+ <ListItem
+ key={entity.id}
+ name={entity.display_name || entity.name}
+ description={entity.description}
+ url={`/reference/metrics/${entity.id}`}
+ icon="ruler"
+ />
),
)}
</List>
diff --git a/frontend/src/metabase/reference/metrics/MetricQuestions.jsx b/frontend/src/metabase/reference/metrics/MetricQuestions.jsx
index fb759fb3409..ef5995f0990 100644
--- a/frontend/src/metabase/reference/metrics/MetricQuestions.jsx
+++ b/frontend/src/metabase/reference/metrics/MetricQuestions.jsx
@@ -82,22 +82,19 @@ class MetricQuestions extends Component {
<div className="wrapper wrapper--trim">
<List>
{Object.values(entities).map(
- (entity, index) =>
+ entity =>
entity &&
entity.id &&
entity.name && (
- <li className="relative" key={entity.id}>
- <ListItem
- id={entity.id}
- index={index}
- name={entity.display_name || entity.name}
- description={t`Created ${moment(
- entity.created_at,
- ).fromNow()} by ${entity.creator.common_name}`}
- url={Urls.question(entity)}
- icon={visualizations.get(entity.display).iconName}
- />
- </li>
+ <ListItem
+ key={entity.id}
+ name={entity.display_name || entity.name}
+ description={t`Created ${moment(
+ entity.created_at,
+ ).fromNow()} by ${entity.creator.common_name}`}
+ url={Urls.question(entity)}
+ icon={visualizations.get(entity.display).iconName}
+ />
),
)}
</List>
diff --git a/frontend/src/metabase/reference/segments/SegmentList.jsx b/frontend/src/metabase/reference/segments/SegmentList.jsx
index ec6644efa88..3a162a8af2f 100644
--- a/frontend/src/metabase/reference/segments/SegmentList.jsx
+++ b/frontend/src/metabase/reference/segments/SegmentList.jsx
@@ -63,20 +63,17 @@ class SegmentList extends Component {
<div className="wrapper wrapper--trim">
<List>
{Object.values(entities).map(
- (entity, index) =>
+ entity =>
entity &&
entity.id &&
entity.name && (
- <li className="relative" key={entity.id}>
- <ListItem
- id={entity.id}
- index={index}
- name={entity.display_name || entity.name}
- description={entity.description}
- url={`/reference/segments/${entity.id}`}
- icon="segment"
- />
- </li>
+ <ListItem
+ key={entity.id}
+ name={entity.display_name || entity.name}
+ description={entity.description}
+ url={`/reference/segments/${entity.id}`}
+ icon="segment"
+ />
),
)}
</List>
diff --git a/frontend/src/metabase/reference/segments/SegmentQuestions.jsx b/frontend/src/metabase/reference/segments/SegmentQuestions.jsx
index d4bd5730099..44459d3f9ea 100644
--- a/frontend/src/metabase/reference/segments/SegmentQuestions.jsx
+++ b/frontend/src/metabase/reference/segments/SegmentQuestions.jsx
@@ -81,22 +81,19 @@ class SegmentQuestions extends Component {
<div className="wrapper wrapper--trim">
<List>
{Object.values(entities).map(
- (entity, index) =>
+ entity =>
entity &&
entity.id &&
entity.name && (
- <li className="relative" key={entity.id}>
- <ListItem
- id={entity.id}
- index={index}
- name={entity.display_name || entity.name}
- description={t`Created ${moment(
- entity.created_at,
- ).fromNow()} by ${entity.creator.common_name}`}
- url={Urls.question(entity)}
- icon={visualizations.get(entity.display).iconName}
- />
- </li>
+ <ListItem
+ key={entity.id}
+ name={entity.display_name || entity.name}
+ description={t`Created ${moment(
+ entity.created_at,
+ ).fromNow()} by ${entity.creator.common_name}`}
+ url={Urls.question(entity)}
+ icon={visualizations.get(entity.display).iconName}
+ />
),
)}
</List>
--
GitLab