Skip to content
Snippets Groups Projects
Unverified Commit 43b596ec authored by Ryan Laurie's avatar Ryan Laurie Committed by GitHub
Browse files

Indexed entity performance improvement (#31825)

 * don't navigate away from the current model if the user searches for an item in that model
parent 90064082
No related branches found
No related tags found
No related merge requests found
......@@ -21,6 +21,7 @@ describe("scenarios > model indexes", () => {
cy.intercept("POST", "/api/model-index").as("modelIndexCreate");
cy.intercept("DELETE", "/api/model-index/*").as("modelIndexDelete");
cy.intercept("PUT", "/api/card/*").as("cardUpdate");
cy.intercept("GET", "/api/card/*").as("cardGet");
cy.createQuestion({
name: "Products Model",
......@@ -169,6 +170,50 @@ describe("scenarios > model indexes", () => {
cy.findAllByText("Anais Zieme").should("have.length", 2);
});
});
it("should not reload the model for record in the same model", () => {
createModelIndex({ modelId, pkName: "ID", valueName: "TITLE" });
cy.visit("/");
cy.findByTestId("app-bar")
.findByPlaceholderText("Search…")
.type("marble shoes");
cy.wait("@searchQuery");
cy.findByTestId("search-results-list")
.findByText("Small Marble Shoes")
.click();
cy.wait("@dataset");
cy.findByTestId("object-detail").within(() => {
cy.findByText("Product");
cy.findByText("Small Marble Shoes");
cy.findByText("Doohickey");
});
// for some reason we hit this endpoint twice on initial load
expectCardQueries(2);
cy.get("body").type("{esc}");
cy.findByTestId("app-bar")
.findByPlaceholderText("Search…")
.clear()
.type("silk coat");
cy.findByTestId("search-results-list")
.findByText("Ergonomic Silk Coat")
.click();
cy.findByTestId("object-detail").within(() => {
cy.findByText("Upton, Kovacek and Halvorson");
});
expectCardQueries(2);
});
});
function editTitleMetadata() {
......@@ -206,3 +251,8 @@ function createModelIndex({ modelId, pkName, valueName }) {
},
);
}
const expectCardQueries = num =>
cy.get("@cardGet.all").then(interceptions => {
expect(interceptions).to.have.length(num);
});
import { MouseEvent, useEffect, useCallback, useRef, useState } from "react";
import _ from "underscore";
import { t } from "ttag";
import { connect } from "react-redux";
import { push } from "react-router-redux";
import { withRouter } from "react-router";
import { Location, LocationDescriptorObject } from "history";
......@@ -14,6 +12,8 @@ import { useOnClickOutside } from "metabase/hooks/use-on-click-outside";
import { useToggle } from "metabase/hooks/use-toggle";
import { isSmallScreen } from "metabase/lib/dom";
import MetabaseSettings from "metabase/lib/settings";
import { useDispatch } from "metabase/lib/redux";
import { zoomInRow } from "metabase/query_builder/actions";
import SearchResults from "./SearchResults";
import RecentsList from "./RecentsList";
......@@ -35,20 +35,12 @@ type RouterProps = {
location: SearchAwareLocation;
};
type DispatchProps = {
onChangeLocation: (nextLocation: LocationDescriptorObject) => void;
};
type OwnProps = {
onSearchActive?: () => void;
onSearchInactive?: () => void;
};
type Props = RouterProps & DispatchProps & OwnProps;
const mapDispatchToProps = {
onChangeLocation: push,
};
type Props = RouterProps & OwnProps;
function isSearchPageLocation(location: Location) {
const components = location.pathname.split("/");
......@@ -62,12 +54,7 @@ function getSearchTextFromLocation(location: SearchAwareLocation) {
return "";
}
function SearchBar({
location,
onSearchActive,
onSearchInactive,
onChangeLocation,
}: Props) {
function SearchBarView({ location, onSearchActive, onSearchInactive }: Props) {
const [searchText, setSearchText] = useState<string>(() =>
getSearchTextFromLocation(location),
);
......@@ -79,6 +66,12 @@ function SearchBar({
const previousLocation = usePrevious(location);
const container = useRef<HTMLDivElement>(null);
const searchInput = useRef<HTMLInputElement>(null);
const dispatch = useDispatch();
const onChangeLocation = useCallback(
(nextLocation: LocationDescriptorObject) => dispatch(push(nextLocation)),
[dispatch],
);
const onInputContainerClick = useCallback(() => {
searchInput.current?.focus();
......@@ -89,6 +82,19 @@ function SearchBar({
setSearchText(e.target.value);
}, []);
const onSearchItemSelect = useCallback(
result => {
// if we're already looking at the right model, don't navigate, just update the zoomed in row
const isSameModel = result?.model_id === location?.state?.cardId;
if (isSameModel && result.model === "indexed-entity") {
zoomInRow({ objectId: result.id })(dispatch);
} else {
onChangeLocation(result.getUrl());
}
},
[dispatch, onChangeLocation, location?.state?.cardId],
);
useOnClickOutside(container, setInactive);
useKeyboardShortcut("Escape", setInactive);
......@@ -181,7 +187,10 @@ function SearchBar({
<SearchResultsFloatingContainer>
{hasSearchText ? (
<SearchResultsContainer>
<SearchResults searchText={searchText.trim()} />
<SearchResults
searchText={searchText.trim()}
onEntitySelect={onSearchItemSelect}
/>
</SearchResultsContainer>
) : (
<RecentsList />
......@@ -191,8 +200,9 @@ function SearchBar({
</SearchBarRoot>
);
}
// eslint-disable-next-line import/no-default-export -- deprecated usage
export default _.compose(
withRouter,
connect(null, mapDispatchToProps),
)(SearchBar);
export const SearchBar = withRouter(SearchBarView);
// for some reason our unit test don't work if this is a name export ¯\_(ツ)_/¯
// eslint-disable-next-line import/no-default-export
export default SearchBar;
......@@ -16,6 +16,7 @@ const propTypes = {
list: PropTypes.array,
onChangeLocation: PropTypes.func,
onEntitySelect: PropTypes.func,
forceEntitySelect: PropTypes.bool,
searchText: PropTypes.string,
};
......@@ -23,6 +24,7 @@ const SearchResults = ({
list,
onChangeLocation,
onEntitySelect,
forceEntitySelect,
searchText,
}) => {
const { reset, getRef, cursorIndex } = useListKeyboardNavigation({
......@@ -42,16 +44,24 @@ const SearchResults = ({
return (
<ul data-testid="search-results-list">
{hasResults ? (
list.map((item, index) => (
<li key={`${item.model}:${item.id}`} ref={getRef(item)}>
<SearchResult
result={item}
compact={true}
isSelected={cursorIndex === index}
onClick={onEntitySelect}
/>
</li>
))
list.map((item, index) => {
const isIndexedEntity = item.model === "indexed-entity";
const onClick =
onEntitySelect && (isIndexedEntity || forceEntitySelect)
? onEntitySelect
: undefined;
return (
<li key={`${item.model}:${item.id}`} ref={getRef(item)}>
<SearchResult
result={item}
compact={true}
isSelected={cursorIndex === index}
onClick={onClick}
/>
</li>
);
})
) : (
<EmptyStateContainer>
<EmptyState message={t`Didn't find anything`} icon="search" />
......
......@@ -73,6 +73,7 @@ export const ResultButton = styled.button<ResultStylesProps>`
padding-right: 0.5rem;
text-align: left;
cursor: pointer;
width: 100%;
&:hover {
${Title} {
color: ${color("brand")};
......
import { color } from "metabase/lib/colors";
import { isSyncCompleted } from "metabase/lib/syncing";
import { Icon } from "metabase/core/components/Icon";
import Text from "metabase/components/type/Text";
......
......@@ -149,6 +149,7 @@ function LinkViz({
<SearchResultsContainer>
<SearchResults
searchText={url?.trim()}
forceEntitySelect
onEntitySelect={handleEntitySelect}
models={MODELS_TO_SEARCH}
/>
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment