From 6f02b515610dbcc6a4191946407b9fade54295d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 24 Apr 2020 19:23:45 +0200 Subject: [PATCH] Search: Improving search look and feel (#23854) * Search: Improving search look and feel * Fixed issue with tag filter beeing cramped and wrapping tags * Minor tag polish * fixed type --- packages/grafana-data/src/types/theme.ts | 1 - .../src/components/Forms/Checkbox.tsx | 4 +- .../Forms/RadioButtonGroup/RadioButton.tsx | 5 +- .../grafana-ui/src/components/Tags/Tag.tsx | 3 +- .../src/components/Tags/TagList.tsx | 3 - packages/grafana-ui/src/themes/dark.ts | 1 - packages/grafana-ui/src/themes/light.ts | 1 - .../core/components/TagFilter/TagBadge.tsx | 5 +- .../core/components/TagFilter/TagFilter.tsx | 6 -- .../core/components/TagFilter/TagOption.tsx | 2 +- .../features/search/components/ActionRow.tsx | 2 +- .../search/components/ManageDashboards.tsx | 4 - .../search/components/SearchCheckbox.tsx | 1 + .../search/components/SearchItem.test.tsx | 10 +-- .../features/search/components/SearchItem.tsx | 52 ++++-------- .../search/components/SearchResults.tsx | 84 +++++++++++-------- .../search/components/SectionHeader.tsx | 15 ++-- public/app/features/search/constants.ts | 3 +- public/sass/components/_tags.scss | 9 +- public/sass/components/_tagsinput.scss | 24 +++--- 20 files changed, 107 insertions(+), 128 deletions(-) diff --git a/packages/grafana-data/src/types/theme.ts b/packages/grafana-data/src/types/theme.ts index 6188762c20e..7751dbfd280 100644 --- a/packages/grafana-data/src/types/theme.ts +++ b/packages/grafana-data/src/types/theme.ts @@ -236,7 +236,6 @@ export interface GrafanaTheme extends GrafanaThemeCommons { formSwitchBgHover: string; formSwitchBgDisabled: string; formSwitchDot: string; - formCheckboxBg: string; formCheckboxBgChecked: string; formCheckboxBgCheckedHover: string; formCheckboxCheckmark: string; diff --git a/packages/grafana-ui/src/components/Forms/Checkbox.tsx b/packages/grafana-ui/src/components/Forms/Checkbox.tsx index 141ea75c3c4..e648ae01c4f 100644 --- a/packages/grafana-ui/src/components/Forms/Checkbox.tsx +++ b/packages/grafana-ui/src/components/Forms/Checkbox.tsx @@ -50,7 +50,7 @@ export const getCheckboxStyles = stylesFactory((theme: GrafanaTheme) => { * */ &:checked + span { background: blue; - background: ${theme.colors.formCheckboxBgChecked}; + background: ${theme.colors.formInputBg}; border: none; &:hover { background: ${theme.colors.formCheckboxBgCheckedHover}; @@ -74,7 +74,7 @@ export const getCheckboxStyles = stylesFactory((theme: GrafanaTheme) => { height: ${checkboxSize}; border-radius: ${theme.border.radius.sm}; margin-right: ${theme.spacing.formSpacingBase}px; - background: ${theme.colors.formCheckboxBg}; + background: ${theme.colors.formInputBg}; border: 1px solid ${theme.colors.formInputBorder}; position: absolute; top: 1px; diff --git a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx index 6f27b4bd851..067173b0e0c 100644 --- a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx +++ b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx @@ -17,7 +17,7 @@ export interface RadioButtonProps { } const getRadioButtonStyles = stylesFactory((theme: GrafanaTheme, size: RadioButtonSize, fullWidth?: boolean) => { - const { fontSize, height } = getPropertiesForButtonSize({ + const { fontSize, height, padding } = getPropertiesForButtonSize({ theme, size, hasIcon: false, @@ -25,7 +25,6 @@ const getRadioButtonStyles = stylesFactory((theme: GrafanaTheme, size: RadioButt variant: 'secondary', }); - const horizontalPadding = theme.spacing[size] ?? theme.spacing.md; const c = theme.palette; const textColor = theme.colors.textSemiWeak; const textColorHover = theme.colors.text; @@ -75,7 +74,7 @@ const getRadioButtonStyles = stylesFactory((theme: GrafanaTheme, size: RadioButt // Deduct border from line-height for perfect vertical centering on windows and linux line-height: ${height - 2}px; color: ${textColor}; - padding: 0 ${horizontalPadding}; + padding: ${padding}; margin-left: -1px; border-radius: ${theme.border.radius.sm}; border: ${border}; diff --git a/packages/grafana-ui/src/components/Tags/Tag.tsx b/packages/grafana-ui/src/components/Tags/Tag.tsx index e2f3581d1cc..8dbb88ce9c3 100644 --- a/packages/grafana-ui/src/components/Tags/Tag.tsx +++ b/packages/grafana-ui/src/components/Tags/Tag.tsx @@ -45,11 +45,10 @@ const getTagStyles = (theme: GrafanaTheme, name: string, colorIndex?: number) => line-height: ${theme.typography.lineHeight.xs}; vertical-align: baseline; background-color: ${colors.color}; - color: ${theme.palette.white}; + color: ${theme.colors.textStrong}; white-space: nowrap; text-shadow: none; padding: 3px 6px; - border: 1px solid ${colors.borderColor}; border-radius: ${theme.border.radius.md}; :hover { diff --git a/packages/grafana-ui/src/components/Tags/TagList.tsx b/packages/grafana-ui/src/components/Tags/TagList.tsx index 4d0db762307..844342ee18e 100644 --- a/packages/grafana-ui/src/components/Tags/TagList.tsx +++ b/packages/grafana-ui/src/components/Tags/TagList.tsx @@ -27,12 +27,9 @@ const getStyles = () => { display: flex; flex: 1 1 auto; flex-wrap: wrap; - padding: 10px; `, tag: css` margin-left: 6px; - font-size: 11px; - padding: 2px 6px; `, }; }; diff --git a/packages/grafana-ui/src/themes/dark.ts b/packages/grafana-ui/src/themes/dark.ts index 678d83be44c..164a126b22c 100644 --- a/packages/grafana-ui/src/themes/dark.ts +++ b/packages/grafana-ui/src/themes/dark.ts @@ -81,7 +81,6 @@ const form = { formSwitchBgActiveHover: basicColors.blue80, formSwitchBgDisabled: basicColors.gray25, formSwitchDot: basicColors.gray15, - formCheckboxBg: basicColors.dark5, formCheckboxBgChecked: basicColors.blue95, formCheckboxBgCheckedHover: basicColors.blue80, formCheckboxCheckmark: basicColors.gray25, diff --git a/packages/grafana-ui/src/themes/light.ts b/packages/grafana-ui/src/themes/light.ts index f3d23b85638..fcc95218837 100644 --- a/packages/grafana-ui/src/themes/light.ts +++ b/packages/grafana-ui/src/themes/light.ts @@ -80,7 +80,6 @@ const form = { formSwitchBgActiveHover: basicColors.blue80, formSwitchBgDisabled: basicColors.gray4, formSwitchDot: basicColors.white, - formCheckboxBg: basicColors.white, formCheckboxBgChecked: basicColors.blue77, formCheckboxBgCheckedHover: basicColors.blue80, formCheckboxCheckmark: basicColors.white, diff --git a/public/app/core/components/TagFilter/TagBadge.tsx b/public/app/core/components/TagFilter/TagBadge.tsx index 6b07285acef..1d071464ff4 100644 --- a/public/app/core/components/TagFilter/TagBadge.tsx +++ b/public/app/core/components/TagFilter/TagBadge.tsx @@ -15,11 +15,12 @@ export class TagBadge extends React.Component { render() { const { label, removeIcon, count } = this.props; - const { color, borderColor } = getTagColorsFromName(label); + const { color } = getTagColorsFromName(label); + const tagStyle = { backgroundColor: color, - borderColor: borderColor, }; + const countLabel = count !== 0 && {`(${count})`}; return ( diff --git a/public/app/core/components/TagFilter/TagFilter.tsx b/public/app/core/components/TagFilter/TagFilter.tsx index b908b9ba724..a3278cb59cd 100644 --- a/public/app/core/components/TagFilter/TagFilter.tsx +++ b/public/app/core/components/TagFilter/TagFilter.tsx @@ -107,17 +107,11 @@ const getStyles = stylesFactory(() => { return { tagFilter: css` min-width: 180px; - line-height: 22px; flex-grow: 1; .label-tag { margin-left: 6px; - font-size: 11px; cursor: pointer; - - .fa.fa-remove { - margin-right: 3px; - } } `, }; diff --git a/public/app/core/components/TagFilter/TagOption.tsx b/public/app/core/components/TagFilter/TagOption.tsx index 68752334da0..1171485b0bf 100644 --- a/public/app/core/components/TagFilter/TagOption.tsx +++ b/public/app/core/components/TagFilter/TagOption.tsx @@ -14,7 +14,7 @@ export const TagOption = (props: ExtendedOptionProps) => { const { data, className, label } = props; return ( -
+
diff --git a/public/app/features/search/components/ActionRow.tsx b/public/app/features/search/components/ActionRow.tsx index a181688f446..45a415bd7e8 100644 --- a/public/app/features/search/components/ActionRow.tsx +++ b/public/app/features/search/components/ActionRow.tsx @@ -44,7 +44,7 @@ export const ActionRow: FC = ({ return (
- + {!hideLayout ? : null} diff --git a/public/app/features/search/components/ManageDashboards.tsx b/public/app/features/search/components/ManageDashboards.tsx index 22f61d4d853..509e038dfbb 100644 --- a/public/app/features/search/components/ManageDashboards.tsx +++ b/public/app/features/search/components/ManageDashboards.tsx @@ -192,10 +192,6 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => { return { container: css` height: 100%; - - .results-container { - padding: 5px 0 0; - } `, searchField: css` height: auto; diff --git a/public/app/features/search/components/SearchCheckbox.tsx b/public/app/features/search/components/SearchCheckbox.tsx index 9fd65c522cb..205fa66bcaf 100644 --- a/public/app/features/search/components/SearchCheckbox.tsx +++ b/public/app/features/search/components/SearchCheckbox.tsx @@ -22,6 +22,7 @@ const getStyles = stylesFactory(() => ({ // Vertically align absolutely positioned checkbox element wrapper: css` height: 21px; + margin-right: 12px; & > label { height: 100%; } diff --git a/public/app/features/search/components/SearchItem.test.tsx b/public/app/features/search/components/SearchItem.test.tsx index 3fb58d82005..aee6d6d8a0c 100644 --- a/public/app/features/search/components/SearchItem.test.tsx +++ b/public/app/features/search/components/SearchItem.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { shallow, mount } from 'enzyme'; +import { mount } from 'enzyme'; import { Tag } from '@grafana/ui'; import { SearchItem, Props } from './SearchItem'; import { DashboardSearchItemType } from '../types'; @@ -17,7 +17,7 @@ const data = { checked: false, }; -const setup = (propOverrides?: Partial, renderMethod = shallow) => { +const setup = (propOverrides?: Partial) => { const props: Props = { item: data, onTagSelected: jest.fn(), @@ -28,7 +28,7 @@ const setup = (propOverrides?: Partial, renderMethod = shallow) => { Object.assign(props, propOverrides); - const wrapper = renderMethod(); + const wrapper = mount(); const instance = wrapper.instance(); return { @@ -39,14 +39,14 @@ const setup = (propOverrides?: Partial, renderMethod = shallow) => { describe('SearchItem', () => { it('should render the item', () => { - const { wrapper } = setup(); + const { wrapper } = setup({}); expect(wrapper.find({ 'aria-label': 'Dashboard search item Test 1' })).toHaveLength(1); expect(wrapper.findWhere(comp => comp.type() === 'div' && comp.text() === 'Test 1')).toHaveLength(1); }); it("should render item's tags", () => { // @ts-ignore - const { wrapper } = setup({}, mount); + const { wrapper } = setup({}); expect(wrapper.find(Tag)).toHaveLength(2); }); }); diff --git a/public/app/features/search/components/SearchItem.tsx b/public/app/features/search/components/SearchItem.tsx index b53ce369e3f..29272301bf6 100644 --- a/public/app/features/search/components/SearchItem.tsx +++ b/public/app/features/search/components/SearchItem.tsx @@ -1,11 +1,11 @@ -import React, { FC, useCallback, useRef, useEffect, CSSProperties } from 'react'; +import React, { FC, useCallback, CSSProperties } from 'react'; import { css, cx } from 'emotion'; import { GrafanaTheme } from '@grafana/data'; import { e2e } from '@grafana/e2e'; -import { Icon, useTheme, TagList, styleMixins, stylesFactory } from '@grafana/ui'; -import { updateLocation } from 'app/core/reducers/location'; +import { useTheme, TagList, styleMixins, stylesFactory } from '@grafana/ui'; import { DashboardSectionItem, OnToggleChecked } from '../types'; import { SearchCheckbox } from './SearchCheckbox'; +import { SEARCH_ITEM_HEIGHT, SEARCH_ITEM_MARGIN } from '../constants'; export interface Props { item: DashboardSectionItem; @@ -20,30 +20,6 @@ const { selectors } = e2e.pages.Dashboards; export const SearchItem: FC = ({ item, editable, onToggleChecked, onTagSelected, style }) => { const theme = useTheme(); const styles = getResultsItemStyles(theme); - const inputEl = useRef(null); - - useEffect(() => { - const preventDef = (event: MouseEvent) => { - // manually prevent default on TagList click, as doing it via normal onClick doesn't work inside angular - event.preventDefault(); - }; - if (inputEl.current) { - inputEl.current.addEventListener('click', preventDef); - } - return () => { - inputEl.current!.removeEventListener('click', preventDef); - }; - }, []); - - const onItemClick = () => { - //Check if one string can be found in the other - if (window.location.pathname.includes(item.url) || item.url.includes(window.location.pathname)) { - updateLocation({ - query: { search: null }, - partial: true, - }); - } - }; const tagSelected = useCallback((tag: string, event: React.MouseEvent) => { onTagSelected(tag); @@ -60,23 +36,21 @@ export const SearchItem: FC = ({ item, editable, onToggleChecked, onTagSe ); return ( -
  • + - -
  • + +
    ); }; @@ -85,8 +59,9 @@ const getResultsItemStyles = stylesFactory((theme: GrafanaTheme) => ({ ${styleMixins.listItem(theme)}; display: flex; align-items: center; - padding: 0 ${theme.spacing.sm}; - min-height: 37px; + height: ${SEARCH_ITEM_HEIGHT}px; + margin-bottom: ${SEARCH_ITEM_MARGIN}px; + padding: 0 ${theme.spacing.md}; :hover { cursor: pointer; @@ -101,7 +76,6 @@ const getResultsItemStyles = stylesFactory((theme: GrafanaTheme) => ({ justify-content: center; flex: 1 1 auto; overflow: hidden; - padding: 0 10px; `, folderTitle: css` color: ${theme.colors.textWeak}; @@ -114,6 +88,7 @@ const getResultsItemStyles = stylesFactory((theme: GrafanaTheme) => ({ margin-left: 10px; `, tags: css` + flex-grow: 0; justify-content: flex-end; @media only screen and (max-width: ${theme.breakpoints.md}) { display: none; @@ -122,6 +97,7 @@ const getResultsItemStyles = stylesFactory((theme: GrafanaTheme) => ({ link: css` display: flex; align-items: center; - width: 100%; + flex-shrink: 0; + flex-grow: 1; `, })); diff --git a/public/app/features/search/components/SearchResults.tsx b/public/app/features/search/components/SearchResults.tsx index 755f28fbcd3..4b2320ed363 100644 --- a/public/app/features/search/components/SearchResults.tsx +++ b/public/app/features/search/components/SearchResults.tsx @@ -1,12 +1,12 @@ import React, { FC } from 'react'; -import { css, cx } from 'emotion'; +import { css } from 'emotion'; import { FixedSizeList } from 'react-window'; import AutoSizer from 'react-virtualized-auto-sizer'; import { GrafanaTheme } from '@grafana/data'; import { stylesFactory, useTheme, Spinner } from '@grafana/ui'; import { DashboardSection, OnToggleChecked, SearchLayout } from '../types'; import { getVisibleItems } from '../utils'; -import { ITEM_HEIGHT } from '../constants'; +import { SEARCH_ITEM_HEIGHT, SEARCH_ITEM_MARGIN } from '../constants'; import { SearchItem } from './SearchItem'; import { SectionHeader } from './SectionHeader'; @@ -35,18 +35,18 @@ export const SearchResults: FC = ({ const renderFolders = () => { return ( -
      +
      {results.map(section => { return ( -
    • +
      -
        +
        {section.expanded && section.items.map(item => )} -
      -
    • +
      +
    ); })} - + ); }; @@ -54,24 +54,26 @@ export const SearchResults: FC = ({ const renderDashboards = () => { return ( - - {({ height }) => ( - - {({ index, style }) => { - const item = items[index]; - return ; - }} - - )} - +
    + + {({ height }) => ( + + {({ index, style }) => { + const item = items[index]; + return ; + }} + + )} + +
    ); }; @@ -80,24 +82,28 @@ export const SearchResults: FC = ({ } else if (!results || !results.length) { return
    No dashboards matching your query were found.
    ; } + return ( -
    - {layout !== SearchLayout.List ? renderFolders() : renderDashboards()} -
    +
    {layout !== SearchLayout.List ? renderFolders() : renderDashboards()}
    ); }; const getSectionStyles = stylesFactory((theme: GrafanaTheme) => { - const { xs, sm, md } = theme.spacing; + const { md } = theme.spacing; + return { wrapper: css` - list-style: none; + display: flex; + flex-direction: column; `, section: css` + display: flex; + flex-direction: column; background: ${theme.colors.panelBg}; - border-bottom: solid 1px ${theme.isLight ? theme.palette.gray95 : theme.palette.gray25}; - padding: 0px ${xs} ${xs}; - margin-bottom: 3px; + border-bottom: solid 1px ${theme.colors.border2}; + `, + sectionItems: css` + margin: 0 24px 0 32px; `, spinner: css` display: flex; @@ -106,14 +112,18 @@ const getSectionStyles = stylesFactory((theme: GrafanaTheme) => { min-height: 100px; `, resultsContainer: css` - padding: ${sm}; position: relative; flex-grow: 10; margin-bottom: ${md}; - background: ${theme.palette.gray10}; - border: 1px solid ${theme.palette.gray15}; + background: ${theme.colors.bg1}; + border: 1px solid ${theme.colors.border1}; border-radius: 3px; height: 100%; `, + listModeWrapper: css` + position: relative; + height: 100%; + padding: ${md}; + `, }; }); diff --git a/public/app/features/search/components/SectionHeader.tsx b/public/app/features/search/components/SectionHeader.tsx index f47d6c6d755..89eafe87500 100644 --- a/public/app/features/search/components/SectionHeader.tsx +++ b/public/app/features/search/components/SectionHeader.tsx @@ -19,7 +19,7 @@ export const SectionHeader: FC = ({ editable = false, }) => { const theme = useTheme(); - const styles = getSectionHeaderStyles(theme, section.selected); + const styles = getSectionHeaderStyles(theme, section.selected, editable); const onSectionExpand = () => { onSectionClick(section); @@ -39,7 +39,10 @@ export const SectionHeader: FC = ({ return (
    - + +
    + +
    {section.title} {section.url && ( @@ -52,15 +55,15 @@ export const SectionHeader: FC = ({ ); }; -const getSectionHeaderStyles = stylesFactory((theme: GrafanaTheme, selected = false) => { - const { sm, xs } = theme.spacing; +const getSectionHeaderStyles = stylesFactory((theme: GrafanaTheme, selected = false, editable: boolean) => { + const { sm } = theme.spacing; return { wrapper: cx( css` display: flex; align-items: center; font-size: ${theme.typography.size.base}; - padding: ${sm} ${xs} ${xs}; + padding: 12px; color: ${theme.colors.textWeak}; &:hover, @@ -78,7 +81,7 @@ const getSectionHeaderStyles = stylesFactory((theme: GrafanaTheme, selected = fa { selected } ), icon: css` - width: 43px; + padding: 0 ${sm} 0 ${editable ? 0 : sm}; `, text: css` flex-grow: 1; diff --git a/public/app/features/search/constants.ts b/public/app/features/search/constants.ts index 7a68dffced8..6b076ab6b0d 100644 --- a/public/app/features/search/constants.ts +++ b/public/app/features/search/constants.ts @@ -1,4 +1,5 @@ export const NO_ID_SECTIONS = ['Recent', 'Starred']; // Height of the search result item -export const ITEM_HEIGHT = 40; +export const SEARCH_ITEM_HEIGHT = 48; +export const SEARCH_ITEM_MARGIN = 4; export const DEFAULT_SORT = { label: 'A-Z', value: 'alpha-asc' }; diff --git a/public/sass/components/_tags.scss b/public/sass/components/_tags.scss index 041bd310d40..8ddb3d80d9a 100644 --- a/public/sass/components/_tags.scss +++ b/public/sass/components/_tags.scss @@ -20,9 +20,14 @@ white-space: nowrap; border-radius: 3px; text-shadow: none; - font-size: 13px; + font-size: 12px; padding: 0px 6px; - border: 1px solid lighten($purple, 10%); + line-height: 20px; + height: 20px; + + svg { + margin-bottom: 0; + } .icon-tag { position: relative; diff --git a/public/sass/components/_tagsinput.scss b/public/sass/components/_tagsinput.scss index 4be68634a42..c325c251d72 100644 --- a/public/sass/components/_tagsinput.scss +++ b/public/sass/components/_tagsinput.scss @@ -46,30 +46,30 @@ } .tag-filter { - line-height: 22px; flex-grow: 1; .label-tag { margin-left: 6px; - font-size: 11px; cursor: pointer; .fa.fa-remove { margin-right: 3px; } } +} - .tag-filter-option { - position: relative; - text-align: left; - width: 100%; - display: block; - border-radius: 0; - } +.tag-filter-option { + position: relative; + text-align: left; + width: 100%; + display: block; + border-radius: 0; + cursor: pointer; + padding: 2px 0; +} - .tag-count-label { - margin-left: 3px; - } +.tag-count-label { + margin-left: 3px; } .tag-filter-values {