Be more consistent with borders and avoid double borders (#41565)

* Be more consistent with borders and avoid double borders

* Mark the aggregation 'Custom Expression' button as an action

* Remove color from action headers

* Remove bottom border from last child

* Remove top border from first child

* Make section header classes consistent

* Add border to search header

* Use correct color for borders

* Add props to enable borders in AccordionList

* Add borders to column picker elements

frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.tsx

@@ -46,6 +46,7 @@ type Section = {
   key: string;
   items: ListItem[];
   icon?: string;
+  type?: string;
 };
 
 function isOperatorListItem(item: ListItem): item is OperatorListItem {
@@ -122,6 +123,7 @@ export function AggregationPicker({
         name: t`Custom Expression`,
         items: [],
         icon: "sum",
+        type: "action",
       });
     }
 
@@ -253,6 +255,7 @@ export function AggregationPicker({
         // disable scrollbars inside the list
         style={{ overflow: "visible" }}
         maxHeight={Infinity}
+        withBorders
       />
     </Root>
   );

frontend/src/metabase/common/components/QueryColumnPicker/QueryColumnPicker.tsx

@@ -203,6 +203,7 @@ export function QueryColumnPicker({
         // Prefer using a11y role selectors
         itemTestId="dimension-list-item"
         globalSearch
+        withBorders
       />
     </DelayGroup>
   );

frontend/src/metabase/core/components/AccordionList/AccordionList.jsx

@@ -100,6 +100,8 @@ export default class AccordionList extends Component {
 
     itemTestId: PropTypes.string,
     "data-testid": PropTypes.string,
+
+    withBorders: PropTypes.bool,
   };
 
   static defaultProps = {
@@ -611,6 +613,7 @@ export default class AccordionList extends Component {
       className,
       sections,
       role,
+      withBorders,
       "data-testid": testId,
     } = this.props;
     const { cursor, scrollToAlignment } = this.state;
@@ -651,6 +654,7 @@ export default class AccordionList extends Component {
               canToggleSections={this.canToggleSections()}
               toggleSection={this.toggleSection}
               hasCursor={this.isRowSelected(rows[index])}
+              withBorders={withBorders}
             />
           ))}
         </AccordionListRoot>
@@ -722,6 +726,7 @@ export default class AccordionList extends Component {
                   sectionIsExpanded={this.isSectionExpanded}
                   canToggleSections={this.canToggleSections()}
                   toggleSection={this.toggleSection}
+                  withBorders={withBorders}
                 />
               )}
             </CellMeasurer>

frontend/src/metabase/core/components/AccordionList/AccordionListCell.jsx

@@ -11,9 +11,9 @@ import CS from "metabase/css/core/index.css";
 import { color } from "metabase/lib/colors";
 import { Icon, Box } from "metabase/ui";
 
+import styles from "./AccordionListCell.module.css";
 import {
   ListCellItem,
-  ListCellHeader,
   FilterContainer,
   Content,
   IconWrapper,
@@ -48,6 +48,7 @@ export const AccordionListCell = ({
   getItemStyles,
   searchInputProps,
   hasCursor,
+  withBorders,
 }) => {
   const {
     type,
@@ -59,6 +60,9 @@ export const AccordionListCell = ({
     isLastSection,
   } = row;
   let content;
+  let borderTop;
+  let borderBottom;
+
   if (type === "header") {
     if (alwaysExpanded) {
       content = (
@@ -79,11 +83,16 @@ export const AccordionListCell = ({
     } else {
       const icon = renderSectionIcon(section);
       const name = section.name;
+
+      borderTop =
+        section.type === "back" ||
+        section.type === "action" ||
+        section.items?.length > 0;
+      borderBottom = section.type === "back";
+
       content = (
-        <ListCellHeader
+        <div
           data-element-id="list-section-header"
-          borderTop={section.type === "back" || section.items?.length > 0}
-          borderBottom={!isLastSection && section.type === "back"}
           className={cx(
             ListS.ListSectionHeader,
             CS.px2,
@@ -136,23 +145,25 @@ export const AccordionListCell = ({
               />
             </span>
           )}
-        </ListCellHeader>
+        </div>
       );
     }
   } else if (type === "action") {
     const icon = renderSectionIcon(section);
     const name = section.name;
+    borderTop = true;
+    borderBottom = !isLastSection;
+
     content = (
-      <ListCellHeader
-        borderTop
-        borderBottom={!isLastSection}
+      <div
         className={cx(
-          "List-section-header",
+          ListS.ListSectionHeader,
           CS.px2,
           CS.py2,
           CS.flex,
           CS.alignCenter,
           CS.hoverParent,
+          styles.action,
           {
             "List-section-header--cursor": hasCursor,
             [CS.cursorPointer]: canToggleSections,
@@ -166,13 +177,23 @@ export const AccordionListCell = ({
       >
         {icon && (
           <span
-            className={cx(CS.mr1, CS.flex, CS.alignCenter, "List-section-icon")}
+            className={cx(
+              CS.mr1,
+              CS.flex,
+              CS.alignCenter,
+              ListS.ListSectionIcon,
+            )}
           >
             {icon}
           </span>
         )}
         {name && (
-          <h3 className={cx("List-section-title", CS.textWrap)}>{name}</h3>
+          <h3
+            data-element-id="list-section-title"
+            className={cx(ListS.ListSectionTitle, CS.textWrap)}
+          >
+            {name}
+          </h3>
         )}
         {showSpinner(section) && (
           <Box ml="0.5rem">
@@ -182,7 +203,7 @@ export const AccordionListCell = ({
         <IconWrapper>
           <Icon name="chevronright" size={12} />
         </IconWrapper>
-      </ListCellHeader>
+      </div>
     );
   } else if (type === "header-hidden") {
     content = <div className={CS.my1} />;
@@ -199,6 +220,7 @@ export const AccordionListCell = ({
       </div>
     );
   } else if (type === "search") {
+    borderBottom = true;
     content = (
       <FilterContainer>
         <ListSearchField
@@ -305,6 +327,8 @@ export const AccordionListCell = ({
       className={cx(section.className, {
         [ListS.ListSectionExpanded]: sectionIsExpanded(sectionIndex),
         [ListS.ListSectionToggleAble]: canToggleSections,
+        [styles.borderTop]: withBorders && borderTop,
+        [styles.borderBottom]: withBorders && borderBottom,
       })}
     >
       {content}

frontend/src/metabase/core/components/AccordionList/AccordionListCell.module.css

+.borderTop {
+  border-top: 1px solid var(--color-border);
+
+  .borderBottom + & {
+    border-top: none;
+  }
+
+  &:first-child {
+    border-top: none;
+  }
+}
+
+.borderBottom {
+  border-bottom: 1px solid var(--color-border);
+
+  &:last-child {
+    border-bottom: none;
+  }
+}
+
+.action {
+  color: var(--color-text-dark);
+
+  &:hover {
+    color: inherit;
+  }
+}

frontend/src/metabase/core/components/AccordionList/AccordionListCell.styled.tsx

 import styled from "@emotion/styled";
 
-import { alpha, color } from "metabase/lib/colors";
+import { alpha } from "metabase/lib/colors";
 
 export interface ListCellItemProps {
   isClickable: boolean;
 }
 
-export const ListCellHeader = styled.div<{
-  borderTop?: boolean;
-  borderBottom?: boolean;
-}>`
-  border: none;
-
-  border: 0px solid ${color("bg-medium")};
-
-  border-bottom-width: ${props => (props.borderBottom ? 1 : 0)}px;
-  border-top-width: ${props => (props.borderTop ? 1 : 0)}px;
-
-  li:first-child & {
-    border-top: none;
-  }
-`;
-
 export const ListCellItem = styled.div<ListCellItemProps>`
   border-color: ${props => props.isClickable && alpha("accent2", 0.2)};
 `;

frontend/src/metabase/query_builder/components/DataSelector/DataSelector.styled.tsx

@@ -95,6 +95,7 @@ export const EmptyStateContainer = styled.div`
 
 export const TableSearchContainer = styled.div`
   padding: 0.5rem;
+  border-bottom: 1px solid ${color("border")};
 `;
 
 export const TriggerContainer = styled.div`

frontend/src/metabase/querying/components/FilterPicker/FilterColumnPicker/FilterColumnPicker.tsx

@@ -128,6 +128,7 @@ export function FilterColumnPicker({
         itemTestId="dimension-list-item"
         searchProp={["name", "displayName"]}
         globalSearch
+        withBorders
       />
     </DelayGroup>
   );