diff --git a/frontend/src/metabase/components/AccordianList.info.js b/frontend/src/metabase/components/AccordianList.info.js new file mode 100644 index 0000000000000000000000000000000000000000..4b056762653d20f6dffc512940a5c40793c39174 --- /dev/null +++ b/frontend/src/metabase/components/AccordianList.info.js @@ -0,0 +1,76 @@ +import React from "react"; +import AccordianList from "metabase/components/AccordianList"; +import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; + +const DemoPopover = ({ children }) => + <PopoverWithTrigger + triggerElement={<button className="Button">Click me!</button>} + verticalAttachments={["top"]} + isInitiallyOpen + > + {children} + </PopoverWithTrigger> + +export const component = AccordianList; + +// disable snapshot testing due to issue with Popover +export const noSnapshotTest = true; + +export const description = ` +An expandable and searchable list of sections and items. +`; + +const sections = [ + { + name: "Widgets", + items: [ + { name: "Foo" }, + { name: "Bar" }, + { name: "Baz" }, + ] + }, + { + name: "Doohickeys", + items: [ + { name: "Buz" }, + ] + } +] + +export const examples = { + "Default": + <DemoPopover> + <AccordianList + className="text-brand" + sections={sections} + itemIsSelected={item => item.name === "Foo"} + /> + </DemoPopover>, + "Always Expanded": + <DemoPopover> + <AccordianList + className="text-brand" + sections={sections} + itemIsSelected={item => item.name === "Foo"} + alwaysExpanded + /> + </DemoPopover>, + "Searchable": + <DemoPopover> + <AccordianList + className="text-brand" + sections={sections} + itemIsSelected={item => item.name === "Foo"} + searchable + /> + </DemoPopover>, + "Hide Single Section Title": + <DemoPopover> + <AccordianList + className="text-brand" + sections={sections.slice(0,1)} + itemIsSelected={item => item.name === "Foo"} + hideSingleSectionTitle + /> + </DemoPopover>, +}; diff --git a/frontend/src/metabase/components/AccordianList.jsx b/frontend/src/metabase/components/AccordianList.jsx index 103832d3c3a33b1f0a67018694a0df541883c43f..2c2bc84ce1ec95c1a7cbc0e447d05e724ec3d570 100644 --- a/frontend/src/metabase/components/AccordianList.jsx +++ b/frontend/src/metabase/components/AccordianList.jsx @@ -1,13 +1,12 @@ import React, { Component } from "react"; import PropTypes from "prop-types"; -import ReactDOM from "react-dom"; import cx from "classnames"; import _ from "underscore"; -import { elementIsInView } from "metabase/lib/dom"; import Icon from "metabase/components/Icon.jsx"; import ListSearchField from "metabase/components/ListSearchField.jsx"; +import { List, CellMeasurer, CellMeasurerCache } from 'react-virtualized'; export default class AccordianList extends Component { constructor(props, context) { @@ -34,6 +33,11 @@ export default class AccordianList extends Component { openSection, searchText: "" }; + + this._cache = new CellMeasurerCache({ + fixedWidth: true, + minHeight: 10, + }); } static propTypes = { @@ -56,6 +60,7 @@ export default class AccordianList extends Component { static defaultProps = { style: {}, + width: 300, searchable: (section) => section.items && section.items.length > 10, alwaysTogglable: false, alwaysExpanded: false, @@ -63,13 +68,54 @@ export default class AccordianList extends Component { }; componentDidMount() { - // when the component is mounted and an item is selected then scroll to it - const element = this.refs.selected && ReactDOM.findDOMNode(this.refs.selected); - if (element && element.scrollIntoView && !elementIsInView(element)) { - element.scrollIntoView(); + // NOTE: for some reason the row heights aren't computed correctly when + // first rendering, so force the list to update + this._forceUpdateList(); + // `scrollToRow` upon mounting, after _forceUpdateList + // Use list.scrollToRow instead of the scrollToIndex prop since the + // causes the list's scrolling to be pinned to the selected row + setTimeout(() => { + if (this._initialSelectedRowIndex != null) { + this._list.scrollToRow(this._initialSelectedRowIndex); + } + }, 0); + } + + componentDidUpdate(prevProps, prevState) { + // if anything changes that affects the selected rows we need to clear the row height cache + if (this.state.openSection !== prevState.openSection || this.state.searchText !== prevState.searchText) { + this._clearRowHeightCache() } } + componentWillUnmount() { + // ensure _forceUpdateList is not called after unmounting + if (this._forceUpdateTimeout != null) { + clearTimeout(this._forceUpdateTimeout); + this._forceUpdateTimeout = null; + } + } + + // resets the row height cache when the displayed rows change + _clearRowHeightCache() { + this._cache.clearAll(); + // NOTE: unclear why this needs to be async + this._forceUpdateTimeout = setTimeout(() => { + this._forceUpdateTimeout = null; + this._forceUpdateList(); + }) + } + + _forceUpdateList() { + // NOTE: unclear why this particular set of functions works, but it does + this._list.invalidateCellSizeAfterRender({ + columnIndex: 0, + rowIndex: 0 + }); + this._list.forceUpdateGrid(); + this.forceUpdate() + } + toggleSection(sectionIndex) { if (this.props.onChangeSection) { if (this.props.onChangeSection(sectionIndex) === false) { @@ -165,90 +211,152 @@ export default class AccordianList extends Component { } render() { - const { id, searchable, searchPlaceholder, sections, showItemArrows, alwaysTogglable, alwaysExpanded, hideSingleSectionTitle, style } = this.props; + const { id, style, searchable, searchPlaceholder, sections, showItemArrows, alwaysTogglable, alwaysExpanded, hideSingleSectionTitle, } = this.props; const { searchText } = this.state; const openSection = this.getOpenSection(); - const sectionIsOpen = (sectionIndex) => + const sectionIsExpanded = (sectionIndex) => alwaysExpanded || openSection === sectionIndex; const sectionIsSearchable = (sectionIndex) => searchable && (typeof searchable !== "function" || searchable(sections[sectionIndex])); + const sectionIsTogglable = (sectionIndex) => + alwaysTogglable || sections.length > 1; - return ( - <div id={id} className={this.props.className} style={{ minWidth: '300px', ...style }}> - {sections.map((section, sectionIndex) => - <section key={sectionIndex} className={cx("List-section", section.className, { "List-section--open": sectionIsOpen(sectionIndex) })}> - { section.name && alwaysExpanded ? - (!hideSingleSectionTitle || sections.length > 1 || alwaysTogglable) && - <div className="px2 pt2 h6 text-grey-2 text-uppercase text-bold"> - {section.name} - </div> - : section.name ? - <div className={"p1 border-bottom"}> - { !hideSingleSectionTitle || sections.length > 1 || alwaysTogglable ? - <div className="List-section-header px1 py1 cursor-pointer full flex align-center" onClick={() => this.toggleSection(sectionIndex)}> - { this.renderSectionIcon(section, sectionIndex) } - <h3 className="List-section-title">{section.name}</h3> - { sections.length > 1 && section.items && section.items.length > 0 && - <span className="flex-align-right"> - <Icon name={sectionIsOpen(sectionIndex) ? "chevronup" : "chevrondown"} size={12} /> - </span> - } - </div> - : - <div className="px1 py1 flex align-center"> - { this.renderSectionIcon(section, sectionIndex) } - <h3 className="text-default">{section.name}</h3> - </div> - } - </div> - : null } - - { sectionIsSearchable(sectionIndex) && sectionIsOpen(sectionIndex) && section.items && section.items.length > 0 && - /* NOTE: much of this structure is here just to match strange stuff in 'List-item' below so things align properly */ - <div className="px1 pt1"> - <div style={{border: "2px solid transparent", borderRadius: "6px"}}> - <ListSearchField - onChange={(val) => this.setState({searchText: val})} - searchText={this.state.searchText} - placeholder={searchPlaceholder} - autoFocus - /> - </div> - </div> + const rows = [] + for (const [sectionIndex, section] of sections.entries()) { + const isLastSection = sectionIndex === sections.length - 1; + if (section.name && (!hideSingleSectionTitle || sections.length > 1 || alwaysTogglable)) { + rows.push({ type: "header", section, sectionIndex, isLastSection }) + } else { + rows.push({ type: "header-hidden", section, sectionIndex, isLastSection }) + } + if (sectionIsSearchable(sectionIndex) && sectionIsExpanded(sectionIndex) && section.items && section.items.length > 0) { + rows.push({ type: "search", section, sectionIndex, isLastSection }) + } + if (sectionIsExpanded(sectionIndex) && section.items && section.items.length > 0) { + for (const [itemIndex, item] of section.items.entries()) { + if (!searchText || item.name.toLowerCase().includes(searchText.toLowerCase())) { + const isLastItem = itemIndex === section.items.length - 1; + if (this.itemIsSelected(item)) { + this._initialSelectedRowIndex = rows.length; } + rows.push({ type: "item", section, sectionIndex, isLastSection, item, itemIndex, isLastItem }); + } + } + } + } - { sectionIsOpen(sectionIndex) && section.items && section.items.length > 0 && - <ul - style={{ maxHeight: alwaysExpanded ? undefined : 400}} - className={cx("p1", { "border-bottom scroll-y scroll-show": !alwaysExpanded })} - > - { section.items.filter((i) => searchText ? (i.name.toLowerCase().includes(searchText.toLowerCase())) : true ).map((item, itemIndex) => - <li - key={itemIndex} - ref={this.itemIsSelected(item, itemIndex) ? "selected" : null} - className={cx("List-item flex", { 'List-item--selected': this.itemIsSelected(item, itemIndex), 'List-item--disabled': !this.itemIsClickable(item) }, this.getItemClasses(item, itemIndex))} - > - <a - className={cx("p1 flex-full flex align-center", this.itemIsClickable(item) ? "cursor-pointer" : "cursor-default")} - onClick={this.itemIsClickable(item) && this.onChange.bind(this, item)} - > - <span className="flex align-center">{ this.renderItemIcon(item, itemIndex) }</span> - <h4 className="List-item-title ml1">{item.name}</h4> - </a> - { this.renderItemExtra(item, itemIndex) } - { showItemArrows && - <div className="List-item-arrow flex align-center px1"> - <Icon name="chevronright" size={8} /> + const maxHeight = (this.props.maxHeight > 0 && this.props.maxHeight < Infinity) ? + this.props.maxHeight : + window.innerHeight; + + const width = this.props.width; + const height = Math.min(maxHeight, rows.reduce((height, row, index) => + height + this._cache.rowHeight({ index }) + , 0)); + + return ( + <List + id={id} + ref={list => this._list = list} + className={this.props.className} + style={style} + width={width} + height={height} + rowCount={rows.length} + + deferredMeasurementCache={this._cache} + rowHeight={this._cache.rowHeight} + + // HACK: needs to be large enough to render enough rows to fill the screen since we used + // the CellMeasurerCache to calculate the height + overscanRowCount={100} + + // ensure `scrollToRow` scrolls the row to the top of the list + scrollToAlignment="start" + + rowRenderer={({ key, index, parent, style }) => { + const { type, section, sectionIndex, item, itemIndex, isLastItem } = rows[index]; + return ( + <CellMeasurer + cache={this._cache} + columnIndex={0} + key={key} + rowIndex={index} + parent={parent} + > + {({ measure }) => + <div + style={style} + className={cx("List-section", section.className, { + "List-section--expanded": sectionIsExpanded(sectionIndex), + "List-section--togglable": sectionIsTogglable(sectionIndex) + })} + > + { type === "header" ? ( + alwaysExpanded ? + <div className="px2 pt2 pb1 h6 text-grey-2 text-uppercase text-bold"> + {section.name} </div> - } - </li> - )} - </ul> - } - </section> - )} - </div> + : + <div + className={cx("List-section-header p2 flex align-center", { + "cursor-pointer": sectionIsTogglable(sectionIndex), + "border-top": sectionIndex !== 0, + "border-bottom": sectionIsExpanded(sectionIndex) + })} + onClick={sectionIsTogglable(sectionIndex) && (() => this.toggleSection(sectionIndex))} + > + { this.renderSectionIcon(section, sectionIndex) } + <h3 className="List-section-title">{section.name}</h3> + { sections.length > 1 && section.items && section.items.length > 0 && + <span className="flex-align-right"> + <Icon name={sectionIsExpanded(sectionIndex) ? "chevronup" : "chevrondown"} size={12} /> + </span> + } + </div> + ) : type === "header-hidden" ? + <div className="my1" /> + : type === "search" ? + <div className="m1" style={{ border: "2px solid transparent" }}> + <ListSearchField + onChange={(val) => this.setState({searchText: val})} + searchText={this.state.searchText} + placeholder={searchPlaceholder} + autoFocus + /> + </div> + : type === "item" ? + <div + className={cx("List-item flex mx1", { + 'List-item--selected': this.itemIsSelected(item), + 'List-item--disabled': !this.itemIsClickable(item), + "mb1": isLastItem + }, this.getItemClasses(item, itemIndex))} + > + <a + className={cx("p1 flex-full flex align-center", this.itemIsClickable(item) ? "cursor-pointer" : "cursor-default")} + onClick={this.itemIsClickable(item) && this.onChange.bind(this, item)} + > + <span className="flex align-center">{ this.renderItemIcon(item, itemIndex) }</span> + <h4 className="List-item-title ml1">{item.name}</h4> + </a> + { this.renderItemExtra(item, itemIndex) } + { showItemArrows && + <div className="List-item-arrow flex align-center px1"> + <Icon name="chevronright" size={8} /> + </div> + } + </div> + : + null + } + </div> + } + </CellMeasurer> + ); + }} + /> ); } } diff --git a/frontend/src/metabase/components/ListSearchField.jsx b/frontend/src/metabase/components/ListSearchField.jsx index 6912ebcca3008eee294abe75d9120de917928e6f..c5371b29a9afb29ee4268c60249ad17ae69afcd1 100644 --- a/frontend/src/metabase/components/ListSearchField.jsx +++ b/frontend/src/metabase/components/ListSearchField.jsx @@ -26,7 +26,7 @@ export default class ListSearchField extends Component { // Call focus() with a small delay because instant input focus causes an abrupt scroll to top of page // when ListSearchField is used inside a popover. It seems that it takes a while for Tether library // to correctly position the popover. - setTimeout(() => this.refs.input.focus(), 50); + setTimeout(() => this._input && this._input.focus(), 50); } } @@ -44,7 +44,7 @@ export default class ListSearchField extends Component { placeholder={placeholder} value={searchText} onChange={(e) => onChange(e.target.value)} - ref="input" + ref={input => this._input = input} /> </div> ); diff --git a/frontend/src/metabase/components/Popover.jsx b/frontend/src/metabase/components/Popover.jsx index 7e61cb22b5c22c7c9ec69355d6e115753007d7b4..1bf668a999a2a923d850dd53d0f525f69e2e414e 100644 --- a/frontend/src/metabase/components/Popover.jsx +++ b/frontend/src/metabase/components/Popover.jsx @@ -15,6 +15,11 @@ import "./Popover.css"; const POPOVER_TRANSITION_ENTER = 100; const POPOVER_TRANSITION_LEAVE = 100; +// space we should leave berween page edge and popover edge +const PAGE_PADDING = 10; +// Popover padding and border +const POPOVER_BODY_PADDING = 2; + export default class Popover extends Component { constructor(props, context) { super(props, context); @@ -102,6 +107,9 @@ export default class Popover extends Component { } _popoverComponent() { + const childProps = { + maxHeight: this._getMaxHeight() + }; return ( <OnClickOutsideWrapper handleDismissal={this.handleDismissal} @@ -122,8 +130,10 @@ export default class Popover extends Component { style={this.props.style} > { typeof this.props.children === "function" ? - this.props.children() - : + this.props.children(childProps) + : React.Children.count(this.props.children) === 1 ? + React.cloneElement(React.Children.only(this.props.children), childProps) + : this.props.children } </div> @@ -147,6 +157,31 @@ export default class Popover extends Component { } } + _getMaxHeight() { + const { top, bottom } = this._getTarget().getBoundingClientRect(); + + let attachments; + if (this.props.pinInitialAttachment && this._best) { + // if we have a pinned attachment only use that + attachments = [this._best.attachmentY] + } else { + // otherwise use the verticalAttachments prop + attachments = this.props.verticalAttachments; + } + + const availableHeights = attachments.map(attachmentY => + attachmentY === "top" ? + window.innerHeight - bottom - this.props.targetOffsetY - PAGE_PADDING + : attachmentY === "bottom" ? + top - this.props.targetOffsetY - PAGE_PADDING + : + 0 + ); + + // get the largest available height, then subtract .PopoverBody's border and padding + return Math.max(...availableHeights) - POPOVER_BODY_PADDING; + } + _getBestAttachmentOptions(tetherOptions, options, attachments, offscreenProps, getAttachmentOptions) { let best = { ...options }; let bestOffScreen = -Infinity; @@ -182,6 +217,32 @@ export default class Popover extends Component { return best; } + _getTarget() { + let target; + if (this.props.targetEvent) { + // create a fake element at the event coordinates + target = document.getElementById("popover-event-target"); + if (!target) { + target = document.createElement("div"); + target.id = "popover-event-target"; + document.body.appendChild(target); + + } + target.style.left = (this.props.targetEvent.clientX - 3) + "px"; + target.style.top = (this.props.targetEvent.clientY - 3) + "px"; + } else if (this.props.target) { + if (typeof this.props.target === "function") { + target = ReactDOM.findDOMNode(this.props.target()); + } else { + target = ReactDOM.findDOMNode(this.props.target); + } + } + if (target == null) { + target = ReactDOM.findDOMNode(this).parentNode; + } + return target; + } + _renderPopover(isOpen) { // popover is open, lets do this! const popoverElement = this._getPopoverElement(); @@ -200,31 +261,10 @@ export default class Popover extends Component { , popoverElement); if (isOpen) { - var tetherOptions = {}; - - tetherOptions.element = popoverElement; - - if (this.props.targetEvent) { - // create a fake element at the event coordinates - tetherOptions.target = document.getElementById("popover-event-target"); - if (!tetherOptions.target) { - tetherOptions.target = document.createElement("div"); - tetherOptions.target.id = "popover-event-target"; - document.body.appendChild(tetherOptions.target); - - } - tetherOptions.target.style.left = (this.props.targetEvent.clientX - 3) + "px"; - tetherOptions.target.style.top = (this.props.targetEvent.clientY - 3) + "px"; - } else if (this.props.target) { - if (typeof this.props.target === "function") { - tetherOptions.target = ReactDOM.findDOMNode(this.props.target()); - } else { - tetherOptions.target = ReactDOM.findDOMNode(this.props.target); - } - } - if (tetherOptions.target == null) { - tetherOptions.target = ReactDOM.findDOMNode(this).parentNode; - } + var tetherOptions = { + element: popoverElement, + target: this._getTarget() + }; if (this.props.tetherOptions) { this._setTetherOptions({ @@ -272,15 +312,14 @@ export default class Popover extends Component { } if (this.props.sizeToFit) { - const verticalPadding = 5; const body = tetherOptions.element.querySelector(".PopoverBody"); if (this._tether.attachment.top === "top") { - if (constrainToScreen(body, "bottom", verticalPadding)) { + if (constrainToScreen(body, "bottom", PAGE_PADDING)) { body.classList.add("scroll-y"); body.classList.add("scroll-show"); } } else if (this._tether.attachment.top === "bottom") { - if (constrainToScreen(body, "top", verticalPadding)) { + if (constrainToScreen(body, "top", PAGE_PADDING)) { body.classList.add("scroll-y"); body.classList.add("scroll-show"); } @@ -313,7 +352,7 @@ export const TestPopover = (props) => // to care about clicks that happen inside the popover onClick={ (e) => { e.stopPropagation(); } } > - { typeof props.children === "function" ? props.children() : props.children} + { typeof props.children === "function" ? props.children({ maxHeight: 500 }) : props.children} </div> </OnClickOutsideWrapper> : null diff --git a/frontend/src/metabase/css/components/list.css b/frontend/src/metabase/css/components/list.css index 21b6f3d7b9d0b9416e1acbabc4f5b86814f96b6f..bae3f575e1de873b91326c693e231c6ef5c29cd4 100644 --- a/frontend/src/metabase/css/components/list.css +++ b/frontend/src/metabase/css/components/list.css @@ -18,15 +18,15 @@ } /* these crazy rules are needed to get currentColor to propagate to the right elements in the right states */ -.List-section .List-section-header:hover, -.List-section .List-section-header:hover .Icon, -.List-section .List-section-header:hover .List-section-title, -.List-section--open .List-section-header, -.List-section--open .List-section-header .List-section-icon .Icon { +.List-section--togglable .List-section-header:hover, +.List-section--togglable .List-section-header:hover .Icon, +.List-section--togglable .List-section-header:hover .List-section-title, +.List-section--expanded .List-section-header, +.List-section--expanded .List-section-header .List-section-icon .Icon { color: currentColor; } -.List-section--open .List-section-header .List-section-title { +.List-section--expanded .List-section-header .List-section-title { color: var(--default-font-color); } diff --git a/frontend/src/metabase/internal/components/ComponentsApp.jsx b/frontend/src/metabase/internal/components/ComponentsApp.jsx index 15e60f1e421ff35f294583abb004bdd64e06190d..07a228f856617a35c88088e620d19a3bcbfad04d 100644 --- a/frontend/src/metabase/internal/components/ComponentsApp.jsx +++ b/frontend/src/metabase/internal/components/ComponentsApp.jsx @@ -1,7 +1,9 @@ import React, { Component } from "react"; +import { Link } from "react-router"; +import { slugify } from "metabase/lib/formatting"; import reactElementToJSXString from "react-element-to-jsx-string"; -import components from "../lib/components-webpack"; +import COMPONENTS from "../lib/components-webpack"; const Section = ({ title, children }) => <div className="mb2"> @@ -11,26 +13,52 @@ const Section = ({ title, children }) => export default class ComponentsApp extends Component { render() { + const componentName = slugify(this.props.params.componentName); + const exampleName = slugify(this.props.params.exampleName); return ( <div className="wrapper p4"> - {components.map(({ component, description, examples }) => ( + {COMPONENTS + .filter(({ component, description, examples }) => + !componentName || componentName === slugify(component.name) + ) + .map(({ component, description, examples }) => ( <div> - <h2>{component.name}</h2> + <h2> + <Link + to={`_internal/components/${slugify(component.name)}`} + className="no-decoration" + > + {component.name} + </Link> + </h2> { description && <p className="my2">{description}</p> } { component.propTypes && <Section title="Props"> - {Object.keys(component.propTypes).map(prop => - <div>{prop}</div> - )} + <div className="border-left border-right border-bottom text-code"> + {Object.keys(component.propTypes).map(prop => + <div>{prop} {component.defaultProps[prop] !== undefined ? "(default: " + JSON.stringify(component.defaultProps[prop]) + ")" : ""}</div> + )} + </div> </Section> } { examples && <Section title="Examples"> - {Object.entries(examples).map(([name, element]) => ( + {Object.entries(examples) + .filter(([name, element]) => + !exampleName || exampleName === slugify(name) + ) + .map(([name, element]) => ( <div className="my2"> - <h4 className="my1">{name}</h4> + <h4 className="my1"> + <Link + to={`_internal/components/${slugify(component.name)}/${slugify(name)}`} + className="no-decoration" + > + {name} + </Link> + </h4> <div className="flex flex-column"> <div className="p2 bordered flex align-center flex-full" diff --git a/frontend/src/metabase/internal/routes.js b/frontend/src/metabase/internal/routes.js index ff49d8e4c6f5de1775ffc5ffcfa1976cb707674a..b1e8626916dbccad7c7c43b69a2259bd44adaa85 100644 --- a/frontend/src/metabase/internal/routes.js +++ b/frontend/src/metabase/internal/routes.js @@ -26,5 +26,7 @@ export default ( { Object.entries(PAGES).map(([name, Component]) => <Route path={name.toLowerCase()} component={Component} /> )} + <Route path="components/:componentName" component={ComponentsApp} /> + <Route path="components/:componentName/:exampleName" component={ComponentsApp} /> </Route> ); diff --git a/frontend/src/metabase/qb/components/gui/BreakoutPopover.jsx b/frontend/src/metabase/qb/components/gui/BreakoutPopover.jsx index 85757d7b3037df360b169382b850afaac65b5148..d27aaf5d58ae2a2969d2df3f4300600d5172b9ee 100644 --- a/frontend/src/metabase/qb/components/gui/BreakoutPopover.jsx +++ b/frontend/src/metabase/qb/components/gui/BreakoutPopover.jsx @@ -8,6 +8,7 @@ import type { Breakout } from "metabase/meta/types/Query"; import type { TableMetadata, FieldOptions } from "metabase/meta/types/Metadata"; type Props = { + maxHeight?: number, breakout?: Breakout, tableMetadata: TableMetadata, fieldOptions: FieldOptions, @@ -21,11 +22,13 @@ const BreakoutPopover = ( tableMetadata, fieldOptions, onCommitBreakout, - onClose + onClose, + maxHeight }: Props ) => ( <FieldList className="text-green" + maxHeight={maxHeight} tableMetadata={tableMetadata} field={breakout} fieldOptions={fieldOptions} diff --git a/frontend/src/metabase/query_builder/components/AggregationPopover.jsx b/frontend/src/metabase/query_builder/components/AggregationPopover.jsx index 2e5da3958cfcd98a2a98c3cc801f93cfa5523991..2033e463e24b4596e68925474ad0ccbd3815d8c9 100644 --- a/frontend/src/metabase/query_builder/components/AggregationPopover.jsx +++ b/frontend/src/metabase/query_builder/components/AggregationPopover.jsx @@ -1,4 +1,5 @@ import React, { Component } from "react"; +import ReactDOM from "react-dom"; import PropTypes from "prop-types"; import { t } from 'c-3po'; import AccordianList from "metabase/components/AccordianList.jsx"; @@ -44,6 +45,14 @@ export default class AggregationPopover extends Component { showOnlyProvidedAggregations: PropTypes.boolean }; + componentDidUpdate() { + if (this._header) { + const { height } = ReactDOM.findDOMNode(this._header).getBoundingClientRect(); + if (height !== this.state.headerHeight) { + this.setState({ headerHeight: height }) + } + } + } commitAggregation(aggregation) { this.props.onCommitAggregation(aggregation); @@ -232,7 +241,7 @@ export default class AggregationPopover extends Component { const [agg, fieldId] = aggregation; return ( <div style={{minWidth: 300}}> - <div className="text-grey-3 p1 py2 border-bottom flex align-center"> + <div ref={_ => this._header = _} className="text-grey-3 p1 py2 border-bottom flex align-center"> <a className="cursor-pointer flex align-center" onClick={this.onClearAggregation}> <Icon name="chevronleft" size={18}/> <h3 className="inline-block pl1">{selectedAggregation.name}</h3> @@ -240,6 +249,7 @@ export default class AggregationPopover extends Component { </div> <FieldList className={"text-green"} + maxHeight={this.props.maxHeight - (this.state.headerHeight || 0)} tableMetadata={tableMetadata} field={fieldId} fieldOptions={query.aggregationFieldOptions(agg)} @@ -253,6 +263,7 @@ export default class AggregationPopover extends Component { return ( <AccordianList className="text-green" + maxHeight={this.props.maxHeight} sections={sections} onChange={this.onPickAggregation} itemIsSelected={this.itemIsSelected.bind(this)} diff --git a/frontend/src/metabase/query_builder/components/DataSelector.jsx b/frontend/src/metabase/query_builder/components/DataSelector.jsx index 68048ade4d8f5291ce1961b5ff7af2ce109ae82a..67e568ace0813818f734bddbfa382b2ee51fccac 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector.jsx +++ b/frontend/src/metabase/query_builder/components/DataSelector.jsx @@ -4,6 +4,7 @@ import { t } from 'c-3po'; import Icon from "metabase/components/Icon.jsx"; import PopoverWithTrigger from "metabase/components/PopoverWithTrigger.jsx"; import AccordianList from "metabase/components/AccordianList.jsx"; +import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper" import { isQueryable } from 'metabase/lib/table'; import { titleize, humanize } from 'metabase/lib/formatting'; @@ -146,9 +147,15 @@ export default class DataSelector extends Component { return this.props.datasetQuery.query && this.props.datasetQuery.query.source_table; } - renderDatabasePicker() { + renderDatabasePicker = ({ maxHeight }) => { + const { databases } = this.state; + + if (databases.length === 0) { + return <LoadingAndErrorWrapper loading />; + } + let sections = [{ - items: this.state.databases.map(database => ({ + items: databases.map(database => ({ name: database.name, database: database })) @@ -157,8 +164,9 @@ export default class DataSelector extends Component { return ( <AccordianList id="DatabasePicker" - key="schemaPicker" + key="databasePicker" className="text-brand" + maxHeight={maxHeight} sections={sections} onChange={this.onChangeTable} itemIsSelected={(item) => this.getDatabaseId() === item.database.id} @@ -168,9 +176,13 @@ export default class DataSelector extends Component { ); } - renderDatabaseSchemaPicker() { + renderDatabaseSchemaPicker = ({ maxHeight }) => { const { databases, selectedSchema } = this.state; + if (databases.length === 0) { + return <LoadingAndErrorWrapper loading />; + } + let sections = databases .map(database => ({ name: database.name, @@ -188,8 +200,9 @@ export default class DataSelector extends Component { <div> <AccordianList id="DatabaseSchemaPicker" - key="schemaPicker" + key="databaseSchemaPicker" className="text-brand" + maxHeight={maxHeight} sections={sections} onChange={this.onChangeSchema} onChangeSection={this.onChangeDatabase} @@ -210,7 +223,7 @@ export default class DataSelector extends Component { ); } - renderSegmentAndDatabasePicker() { + renderSegmentAndDatabasePicker = ({ maxHeight }) => { const { selectedSchema } = this.state; const segmentItem = [{ name: 'Segments', items: [], icon: 'segment'}]; @@ -230,8 +243,10 @@ export default class DataSelector extends Component { return ( <AccordianList - key="schemaPicker" + id="SegmentAndDatabasePicker" + key="segmentAndDatabasePicker" className="text-brand" + maxHeight={maxHeight} sections={sections} onChange={this.onChangeSchema} onChangeSection={(index) => index === 0 ? @@ -248,7 +263,7 @@ export default class DataSelector extends Component { ); } - renderTablePicker() { + renderTablePicker = ({ maxHeight }) => { const schema = this.state.selectedSchema; const isSavedQuestionList = schema.database.is_saved_questions; @@ -297,16 +312,16 @@ export default class DataSelector extends Component { id="TablePicker" key="tablePicker" className="text-brand" + maxHeight={maxHeight} sections={sections} - searchable={true} + searchable onChange={this.onChangeTable} itemIsSelected={(item) => item.table ? item.table.id === this.getTableId() : false} itemIsClickable={(item) => item.table && !item.disabled} renderItemIcon={(item) => item.table ? <Icon name="table2" size={18} /> : null} - hideSingleSectionTitle={true} /> { isSavedQuestionList && ( - <div className="bg-slate-extra-light p2 text-centered"> + <div className="bg-slate-extra-light p2 text-centered border-top"> {t`Is a question missing?`} <a href="http://metabase.com/docs/latest/users-guide/04-asking-questions.html#source-data" className="block link">{t`Learn more about nested queries`}</a> </div> @@ -316,8 +331,8 @@ export default class DataSelector extends Component { } } - //TODO: refactor this. lots of shared code with renderTablePicker() - renderSegmentPicker() { + //TODO: refactor this. lots of shared code with renderTablePicker = () => + renderSegmentPicker = ({ maxHeight }) => { const { segments } = this.props; const header = ( <span className="flex align-center"> @@ -353,10 +368,12 @@ export default class DataSelector extends Component { return ( <AccordianList + id="SegmentPicker" key="segmentPicker" className="text-brand" + maxHeight={maxHeight} sections={sections} - searchable={true} + searchable searchPlaceholder={t`Find a segment`} onChange={this.onChangeSegment} itemIsSelected={(item) => item.segment ? item.segment.id === this.getSegmentId() : false} @@ -411,21 +428,20 @@ export default class DataSelector extends Component { <PopoverWithTrigger id="DataPopover" ref="popover" - sizeToFit isInitiallyOpen={this.props.isInitiallyOpen} triggerElement={triggerElement} triggerClasses="flex align-center" horizontalAttachments={this.props.segments ? ["center", "left", "right"] : ["left"]} > { !this.props.includeTables ? - this.renderDatabasePicker() : + this.renderDatabasePicker : this.state.selectedSchema && this.state.showTablePicker ? - this.renderTablePicker() : + this.renderTablePicker : this.props.segments ? this.state.showSegmentPicker ? - this.renderSegmentPicker() : - this.renderSegmentAndDatabasePicker() : - this.renderDatabaseSchemaPicker() + this.renderSegmentPicker : + this.renderSegmentAndDatabasePicker : + this.renderDatabaseSchemaPicker } </PopoverWithTrigger> ); diff --git a/frontend/src/metabase/query_builder/components/FieldList.jsx b/frontend/src/metabase/query_builder/components/FieldList.jsx index fffe81bc9a6100ad3817df5b356b5917676006db..09cc3bc3c2ded170818743c5a2889cc9b9525f7c 100644 --- a/frontend/src/metabase/query_builder/components/FieldList.jsx +++ b/frontend/src/metabase/query_builder/components/FieldList.jsx @@ -26,6 +26,7 @@ export type AccordianListSection = { type Props = { className?: string, + maxHeight?: number, field: ?ConcreteField, onFieldChange: (field: ConcreteField) => void, @@ -208,6 +209,7 @@ export default class FieldList extends Component { return ( <AccordianList className={this.props.className} + maxHeight={this.props.maxHeight} sections={this.state.sections} onChange={this.onChange} itemIsSelected={this.itemIsSelected} diff --git a/frontend/src/metabase/query_builder/components/filters/FilterPopover.jsx b/frontend/src/metabase/query_builder/components/filters/FilterPopover.jsx index d6f1f6e78910b8a2722323e0d4bd3729fa2b1fe5..ef7c15238d11feed0e822599ccbd4ec5750c44bc 100644 --- a/frontend/src/metabase/query_builder/components/filters/FilterPopover.jsx +++ b/frontend/src/metabase/query_builder/components/filters/FilterPopover.jsx @@ -23,6 +23,7 @@ import type { Filter, FieldFilter, ConcreteField } from "metabase/meta/types/Que import type { FieldMetadata, Operator } from "metabase/meta/types/Metadata"; type Props = { + maxHeight?: number, query: StructuredQuery, filter?: Filter, onCommitFilter: (filter: Filter) => void, @@ -246,6 +247,7 @@ export default class FilterPopover extends Component { <div className="FilterPopover"> <FieldList className="text-purple" + maxHeight={this.props.maxHeight} field={fieldRef} fieldOptions={query.filterFieldOptions(filter)} segmentOptions={query.filterSegmentOptions(filter)} diff --git a/frontend/test/admin/datamodel/FieldApp.integ.spec.js b/frontend/test/admin/datamodel/FieldApp.integ.spec.js index d38778f4c088dbb857a7e03b84a52e38c1309542..793168b92535f83247eeb34e2d7b7258bfd3b1d4 100644 --- a/frontend/test/admin/datamodel/FieldApp.integ.spec.js +++ b/frontend/test/admin/datamodel/FieldApp.integ.spec.js @@ -264,7 +264,7 @@ describe("FieldApp", () => { click(fkFieldSelect); const sourceField = fkFieldSelect.parent().find(TestPopover) - .find("li") + .find(".List-item") .filterWhere(li => /Source/.test(li.text())) .first().children().first(); @@ -385,7 +385,7 @@ describe("FieldApp", () => { store.waitForActions([UPDATE_FIELD_VALUES]); }); - + it("shows the updated values after page reload", async () => { const { fieldApp } = await initFieldApp({ tableId: PRODUCT_RATING_TABLE_ID, fieldId: PRODUCT_RATING_ID }); const section = fieldApp.find(FieldRemapping) @@ -417,4 +417,4 @@ describe("FieldApp", () => { }) }) -}) \ No newline at end of file +}) diff --git a/frontend/test/components/AccordianList.unit.test.js b/frontend/test/components/AccordianList.unit.test.js new file mode 100644 index 0000000000000000000000000000000000000000..ee159d9e228455d401405de3dbeef0bdfed5cc79 --- /dev/null +++ b/frontend/test/components/AccordianList.unit.test.js @@ -0,0 +1,71 @@ +import React from "react"; +import { mount } from "enzyme"; + +import AccordianList from "metabase/components/AccordianList"; +import ListSearchField from "metabase/components/ListSearchField"; + +const SECTIONS = [ + { + name: "Widgets", + items: [{ name: "Foo" }, { name: "Bar" }] + }, + { + name: "Doohickeys", + items: [{ name: "Baz" }] + } +]; + +describe("AccordianList", () => { + it("should open the first section by default", () => { + const wrapper = mount(<AccordianList sections={SECTIONS} />); + expect(wrapper.find(".List-section-header").length).toBe(2); + expect(wrapper.find(".List-item").length).toBe(2); + }); + it("should open the second section if initiallyOpenSection=1", () => { + const wrapper = mount( + <AccordianList sections={SECTIONS} initiallyOpenSection={1} /> + ); + expect(wrapper.find(".List-item").length).toBe(1); + }); + it("should not open a section if initiallyOpenSection=null", () => { + const wrapper = mount( + <AccordianList sections={SECTIONS} initiallyOpenSection={null} /> + ); + expect(wrapper.find(".List-item").length).toBe(0); + }); + it("should open all sections if alwaysExpanded is set", () => { + const wrapper = mount( + <AccordianList sections={SECTIONS} alwaysExpanded /> + ); + expect(wrapper.find(".List-item").length).toBe(3); + }); + it("should not show search field by default", () => { + const wrapper = mount(<AccordianList sections={SECTIONS} />); + expect(wrapper.find(ListSearchField).length).toBe(0); + }); + it("should show search field is searchable is set", () => { + const wrapper = mount(<AccordianList sections={SECTIONS} searchable />); + expect(wrapper.find(ListSearchField).length).toBe(1); + }); + it("should close the section when header is clicked", () => { + const wrapper = mount(<AccordianList sections={SECTIONS} />); + expect(wrapper.find(".List-item").length).toBe(2); + wrapper.find(".List-section-header").first().simulate('click'); + expect(wrapper.find(".List-item").length).toBe(0); + }); + it("should switch sections when another section is clicked", () => { + const wrapper = mount(<AccordianList sections={SECTIONS} />); + expect(wrapper.find(".List-item").length).toBe(2); + wrapper.find(".List-section-header").last().simulate('click'); + expect(wrapper.find(".List-item").length).toBe(1); + }); + it("should filter items when searched", () => { + const wrapper = mount(<AccordianList sections={SECTIONS} searchable />); + const searchInput = wrapper.find(ListSearchField).find("input"); + expect(wrapper.find(".List-item").length).toBe(2); + searchInput.simulate("change", { target: { value: "Foo" }}) + expect(wrapper.find(".List-item").length).toBe(1); + searchInput.simulate("change", { target: { value: "Something Else" }}) + expect(wrapper.find(".List-item").length).toBe(0); + }); +}); diff --git a/frontend/test/internal/components.unit.spec.js b/frontend/test/internal/components.unit.spec.js index 3320d12a0cd708c905d67f3401cefc0ce18cd2f0..e572657b0c37751b8e633a0041e00f5f936e32d8 100644 --- a/frontend/test/internal/components.unit.spec.js +++ b/frontend/test/internal/components.unit.spec.js @@ -3,8 +3,8 @@ import renderer from "react-test-renderer"; import components from "metabase/internal/lib/components-node"; // generates a snapshot test for every example in every component's `.info.js` -components.map(({ component, examples }) => - describe(component.displayName, () => { +components.map(({ component, examples, noSnapshotTest }) => + !noSnapshotTest && describe(component.displayName, () => { Object.entries(examples).map(([exampleName, element]) => it(`should render "${exampleName}" correctly`, () => { expect(renderer.create(element).toJSON()).toMatchSnapshot(); diff --git a/frontend/test/query_builder/components/FieldList.integ.spec.js b/frontend/test/query_builder/components/FieldList.integ.spec.js index e779acf5969718a045e4a7aec3fcedf47ea01228..2bac80cb4d80d0d462fbaffcb088a0302a264921 100644 --- a/frontend/test/query_builder/components/FieldList.integ.spec.js +++ b/frontend/test/query_builder/components/FieldList.integ.spec.js @@ -70,16 +70,16 @@ describe('FieldList', () => { // Maybe also TestTooltip should provide an interface (like `tooltipWrapper.instance().show()`) for toggling it? const tooltipTarget = component.find(`.List-item-title[children="${segment.name}"]`) .first() - .closest('li') + .closest('.List-item') .find(".QuestionTooltipTarget") .parent(); tooltipTarget.simulate("mouseenter"); - + const tooltipContent = tooltipTarget.closest(TestTooltip).find(TestTooltipContent); expect(tooltipContent.length).toBe(1) // eslint-disable-next-line no-irregular-whitespace expect(tooltipContent.find(FilterWidget).last().text()).toMatch(/Created At -300day/); }) -}); \ No newline at end of file +});