Commit 61058dfb authored by Mohammad Nakhaee's avatar Mohammad Nakhaee
Browse files

Merge branch '950-gui-error-when-clicking-in-in-filters-in-entries-search' into 'develop'

Resolve "GUI error when clicking in "+" in filters in entries search."

Closes #950

See merge request !758
parents 9f63f1ee 84c92ddb
Pipeline #138732 passed with stages
in 86 minutes and 1 second
......@@ -33,8 +33,6 @@
"material-ui-chip-input": "^1.1.0",
"material-ui-flat-pagination": "^4.0.0",
"mathjs": "^7.1.0",
"muuri": "^0.9.5",
"muuri-react": "^3.1.6",
"object-hash": "^3.0.0",
"pace": "^0.0.4",
"pace-js": "^1.0.2",
......@@ -48,6 +46,7 @@
"react-detectable-overflow": "^0.6.2",
"react-dom": "^17.0.2",
"react-dropzone": "^5.0.1",
"react-grid-layout": "^1.3.4",
"react-highlight": "^0.14.0",
"react-infinite-scroller": "^1.2.4",
"react-json-view": "^1.19.1",
......@@ -55,7 +54,7 @@
"react-mathjax": "^1.0.1",
"react-pdf": "^5.7.2",
"react-resize-detector": "^7.1.2",
"react-router-cache-route": "^1.11.1",
"react-router-cache-route": "^1.12.11",
"react-router-dom": "^5.1.2",
"react-scripts": "4.0.3",
"react-swipeable-views": "^0.14.0",
......@@ -105,11 +104,11 @@
},
"browserslist": {
"production": [
">0.2%",
"not dead",
"not op_mini all",
"not IE <= 11",
"not Android <= 4.4"
">0.2%",
"not dead",
"not op_mini all",
"not IE <= 11",
"not Android <= 4.4"
],
"development": [
"last 1 chrome version",
......@@ -125,8 +124,7 @@
"text-summary",
"cobertura"
],
"transformIgnorePatterns": [
],
"transformIgnorePatterns": [],
"moduleNameMapper": {
"^.+\\.(css|less)$": "<rootDir>/src/CSSStub.js",
"three.meshline": "<rootDir>/src/CSSStub.js"
......
......@@ -113,7 +113,8 @@ export const Action = React.memo(({
TooltipProps,
className,
classes,
children
children,
'data-testid': testID
}) => {
const styles = useActionStyles({classes: classes})
......@@ -129,6 +130,7 @@ export const Action = React.memo(({
disabled={disabled}
href={href}
aria-label={tooltip}
data-testid={testID}
>
{children}
</IconButton>
......@@ -143,6 +145,7 @@ export const Action = React.memo(({
disabled={disabled}
href={href}
aria-label={tooltip}
data-testid={testID}
>
{children}
</Button>
......@@ -163,7 +166,8 @@ Action.propTypes = {
TooltipProps: PropTypes.object,
className: PropTypes.string,
classes: PropTypes.object,
children: PropTypes.node
children: PropTypes.node,
'data-testid': PropTypes.string
}
Action.defaultProps = {
......
......@@ -19,6 +19,7 @@ import React from 'react'
import { Router, Route } from 'react-router-dom'
import { QueryParamProvider } from 'use-query-params'
import { RecoilRoot } from 'recoil'
import CssBaseline from '@material-ui/core/CssBaseline'
import history from '../history'
import DateFnsUtils from '@date-io/date-fns'
import { MuiPickersUtilsProvider } from '@material-ui/pickers'
......@@ -49,6 +50,7 @@ export default function App() {
<Router history={history}>
<QueryParamProvider ReactRouterRoute={Route}>
<MuiThemeProvider theme={nomadTheme}>
<CssBaseline />
<ErrorSnacks>
<ErrorBoundary>
<GlobalLoginRequired>
......
......@@ -31,7 +31,6 @@ import { atom, atomFamily, useRecoilState, useRecoilCallback, useRecoilValue } f
const presets = {
new: {
align: 'right',
icon: 'filled',
iconSize: 'small',
menu: 'visible',
......@@ -41,7 +40,6 @@ const presets = {
aggCollapse: 'off'
},
old: {
align: 'right',
icon: 'plain',
iconSize: 'medium',
menu: 'hidden',
......@@ -100,7 +98,6 @@ const useStyles = makeStyles(theme => ({
const GUIMenu = React.memo(() => {
const styles = useStyles()
const [visible, setVisible] = useState(false)
const [align, setAlign] = useRecoilState(guiState('align'))
const [icon, setIcon] = useRecoilState(guiState('icon'))
const [iconSize, setIconSize] = useRecoilState(guiState('iconSize'))
const [menu, setMenu] = useRecoilState(guiState('menu'))
......@@ -191,13 +188,6 @@ const GUIMenu = React.memo(() => {
tooltip="Controls the display style for filter settings that include e.g. statistics scaling."
options={['hidden', 'visible']}
/>
<GUIMenuItem
title="Statistics icon alignment"
value={align}
onChange={setAlign}
tooltip="Controls the alignment of the icon that is used to add/remove a filter from the statistics grid."
options={['left', 'right']}
/>
<GUIMenuItem
title="Statistics icon style"
value={icon}
......
......@@ -432,6 +432,8 @@ export const Routes = React.memo(function Routes() {
component={route.component}
render={route.render}
when={route.cache}
unmount={false}
saveScrollPosition={false}
className={route.cache && styles.wrapper}
>
{route.children || undefined}
......
......@@ -225,10 +225,10 @@ function registerFilterOptions(name, group, target, label, description, options)
/**
* Tries to automatically create a suitable statistics component for the given
* quantity.
* @param {string} quantity
* @param {bool} nested
* @param {DType} dtype
* @returns A statistics setup.
*
* @param {string} parent Parent quantity
* @param {DType} dtype Datatype of the quantity
* @returns A statistics setup including the component and a layout.
*/
const getStatsComponent = (parent, dtype) => {
const section = filterData?.[parent]?.section
......@@ -249,16 +249,16 @@ const getStatsComponent = (parent, dtype) => {
return {
component: wrap(InputRange),
layout: {
width: 'medium',
ratio: 3 / 1
width: 8,
height: 3
}
}
} else {
return {
component: wrap(InputList),
layout: {
width: 'small',
ratio: 3 / 4
width: 6,
height: 8
}
}
}
......@@ -267,8 +267,8 @@ const getStatsComponent = (parent, dtype) => {
const ptStatConfig = {
component: InputPeriodicTable,
layout: {
width: 'large',
ratio: 3 / 2
width: 12,
height: 8
}
}
......
......@@ -36,7 +36,8 @@ import {
isSet,
isFunction,
size,
isEqual
isEqual,
merge
} from 'lodash'
import qs from 'qs'
import PropTypes from 'prop-types'
......@@ -103,6 +104,7 @@ export const SearchContext = React.memo(({
resource,
filtersLocked,
initialPagination,
initialStatistics,
children
}) => {
const {api} = useApi()
......@@ -134,7 +136,7 @@ export const SearchContext = React.memo(({
// Initialize the search context state: any parameters in the URL are read and
// default values as specified in filter registry are loaded
const [initialQuery, initialAggs, initialStatistics, filterDefaults] = useMemo(() => {
const [initialQuery, initialAggs, finalStatistics, filterDefaults] = useMemo(() => {
const filterDefaults = {}
for (const [key, value] of Object.entries(filterData)) {
if (!isNil(value.default)) {
......@@ -150,6 +152,9 @@ export const SearchContext = React.memo(({
[queryURL, statisticsURL] = qsToSearch(qs)
}
// Any statistics given in the URL override the initial statistics
const finalStatistics = merge(initialStatistics || {}, statisticsURL)
const initialAggs = {}
for (const key of filters) {
initialAggs[key] = {
......@@ -159,10 +164,10 @@ export const SearchContext = React.memo(({
return [
{...queryURL, ...toGUIFilter(filtersLocked)},
initialAggs,
statisticsURL,
finalStatistics,
filterDefaults
]
}, [filterData, filters, filtersLocked])
}, [filterData, filters, filtersLocked, initialStatistics])
// Initialize a bunch of Recoil.js states and hooks. Notice how we are not using a set
// of global states, but instead each SearchContext gets it's own states. This
......@@ -293,7 +298,7 @@ export const SearchContext = React.memo(({
const statisticFamily = atomFamily({
key: `statisticFamily_${indexContext}`,
default: (name) => initialStatistics[name]
default: (name) => finalStatistics[name]
})
// Used to get/set the the statistics configuration of all filters
......@@ -737,7 +742,7 @@ export const SearchContext = React.memo(({
useAgg,
useSetFilters
]
}, [initialQuery, filters, filtersLocked, initialStatistics, initialAggs, initialPagination, resource, filterData])
}, [initialQuery, filters, filtersLocked, finalStatistics, initialAggs, initialPagination, resource, filterData])
const setResults = useSetRecoilState(resultsState)
const setApiData = useSetRecoilState(apiDataState)
......@@ -1067,6 +1072,7 @@ SearchContext.propTypes = {
resource: PropTypes.string,
filtersLocked: PropTypes.object,
initialPagination: PropTypes.object,
initialStatistics: PropTypes.object,
children: PropTypes.node
}
......@@ -1092,7 +1098,7 @@ function qsToSearch(queryString) {
const stats = queryObj.statistics
if (stats) {
const statsArray = isArray(stats) ? stats : [stats]
statsArray.forEach((name, i) => { statistics[name] = {index: i} })
statsArray.forEach((name, i) => { statistics[name] = {index: Date.now() + i} })
delete queryObj.statistics
}
......
......@@ -18,10 +18,15 @@
import React from 'react'
import PropTypes from 'prop-types'
import assert from 'assert'
import { waitFor, waitForElementToBeRemoved } from '@testing-library/dom'
import elementData from '../../elementData.json'
import { screen, WrapperDefault, WrapperNoAPI } from '../conftest.spec'
import { render } from '@testing-library/react'
import { SearchContext } from './SearchContext'
import { filterData } from './FilterRegistry'
import { format } from 'date-fns'
import { DType } from '../../utils'
/*****************************************************************************/
// Renders
......@@ -70,14 +75,145 @@ export const renderNoAPISearchEntry = (ui, options) =>
* @param {boolean} disableScale Is the statistics scaling is disabled.
* @param {object} root The container to work on.
*/
export function expectInputHeader(quantity, disableScale, root = screen) {
export async function expectInputHeader(quantity, disableScale, root = screen) {
const data = filterData[quantity]
const label = data.label
const description = data.description
expect(root.getByText(label, {exact: false})).toBeInTheDocument()
expect(await root.findByText(label, {exact: false})).toBeInTheDocument()
expect(root.getByTitle(description)).toBeInTheDocument()
if (!disableScale) {
const scale = data.scale
expect(root.getByText(scale)).toBeInTheDocument()
}
}
/**
* Tests that an InputList is rendered with the given contents.
* @param {string} quantity The quantity name
* @param {bool} loaded Whether the data is already loaded.
* @param {string[]} items List of items to be displayed
* @param {string} prompt The prompt to show at the end. One of 'all', 'first'.
* If the given list of items is empty, this prompt is ignored.
*/
export async function expectInputList(quantity, loaded, items, prompt, root = screen) {
const prompts = new Set(['all', 'top', 'single'])
assert(
items.length === 0 || prompts.has(prompt),
`Please provide one of the values: ${[...prompts].join(', ')}`
)
assert(
prompt !== 'single' || items.length === 1,
'Only provide one value with prompt=single'
)
// Test immediately displayed elements
await expectInputHeader(quantity)
// Check that placeholder disappears
!loaded && await waitForElementToBeRemoved(() => root.getByTestId('inputlist-placeholder'))
// Test elements that are displayed after API response
for (const item of items) {
expect(await root.findByText(item)).toBeInTheDocument()
}
// Expect a message at the end
if (items.length === 0) {
expect(root.getByText('No options available with current query.')).toBeInTheDocument()
} else if (prompt === 'all') {
expect(root.getByText(`Showing all ${items.length} items`)).toBeInTheDocument()
} else if (prompt === 'top') {
expect(root.getByText(`Showing top ${items.length} items`)).toBeInTheDocument()
} else if (prompt === 'single') {
expect(root.getByText(`Showing the only item`)).toBeInTheDocument()
}
}
/**
* Tests that an InputRange is rendered with the given contents.
* @param {string} quantity The quantity name
* @param {bool} loaded Whether the data is already loaded
* @param {bool} histogram Whether the histogram is shown
* @param {bool} placeholder Whether the placeholder should be checked
*/
export async function expectInputRange(quantity, loaded, histogram, anchored, min, max, root = screen) {
// Test header
await expectInputHeader(quantity, true)
// Check histogram
if (histogram) {
// Check that placeholder disappears
!loaded && await waitForElementToBeRemoved(() => root.getByTestId('inputrange-histogram-placeholder'))
}
// Test text elements if the component is not anchored
if (!anchored) {
const data = filterData[quantity]
const dtype = data.dtype
if (dtype === DType.Timestamp) {
expect(root.getByText('Start time')).toBeInTheDocument()
expect(root.getByText('End time')).toBeInTheDocument()
} else {
expect(root.getByText('min')).toBeInTheDocument()
expect(root.getByText('max')).toBeInTheDocument()
}
// Get the formatted datetime in current timezone (timezones differ, so the
// local timezone must be used in order to prevent tests from breaking).
if (dtype === DType.Timestamp) {
min = format(min, 'dd/MM/yyyy kk:mm')
max = format(max, 'dd/MM/yyyy kk:mm')
}
// Test elements that are displayed after API response
expect(await root.findByDisplayValue(min)).toBeInTheDocument()
expect(await root.findByDisplayValue(max)).toBeInTheDocument()
}
}
/**
* Tests that an InputPeriodicTable is rendered with the given contents.
* @param {string} quantity The quantity name
* @param {bool} loaded Whether the data is already loaded.
* @param {array} elements List of chemical symbols.
* @param {object} root The root element to perform the search on.
*/
export async function expectInputPeriodicTable(quantity, loaded, elements, root = screen) {
// Test header
await expectInputHeader(quantity)
// Test that all elements are displayed
elementData.elements.forEach(element => {
const name = root.getByText(element.symbol)
expect(name).toBeInTheDocument()
expect(root.getAllByText(element.number)) // This number may also be used as a count
expect(root.getByTitle(element.name)).toBeInTheDocument()
if (!loaded) {
expect(name.closest('button')).toHaveAttribute('disabled')
}
})
expect(screen.getByRole('checkbox')).toBeInTheDocument()
// Test that only available elements are clickable after API response.
await expectInputPeriodicTableItems(elements)
}
/**
* Tests that an InputPeriodicTable has the given elements available.
* @param {array} elements List of chemical symbols.
* @param {object} root The root element to perform the search on.
*/
export async function expectInputPeriodicTableItems(elements, root = screen) {
// Test that only available elements are clickable after API response.
const elementSet = new Set(elements)
await waitFor(() => {
elementData.elements.forEach(element => {
const button = root.getByText(element.symbol).closest('button')
if (elementSet.has(element.symbol)) {
expect(button).not.toHaveAttribute('disabled')
} else {
expect(button).toHaveAttribute('disabled')
}
})
})
}
......@@ -48,7 +48,7 @@ describe('', () => {
test('initial state is loaded correctly for quantity with fixed options', async () => {
// Test immediately displayed elements
const allOptions = getAllOptions(quantity)
expectInputHeader(quantity)
await expectInputHeader(quantity)
for (const option of allOptions) {
expect(await screen.findByText(option)).toBeInTheDocument()
}
......@@ -71,9 +71,6 @@ describe('', () => {
afterEach(() => closeAPI())
test('initial state is loaded correctly for quantity with dynamically loaded options', async () => {
// Test immediately displayed elements
expectInputHeader(quantity)
// Test that placeholder is shown while loading
const placeholder = screen.queryByTestId('inputfield-placeholder')
expect(placeholder).toBeInTheDocument()
......@@ -81,6 +78,9 @@ describe('', () => {
// Check that placeholder disappears
await waitForElementToBeRemoved(() => screen.queryByTestId('inputfield-placeholder'))
// Test header
await expectInputHeader(quantity)
// Test that options become selectable after API call finishes
await expectOptions(['VASP', 'exciting'], optionsProgramName)
......@@ -108,7 +108,7 @@ describe('', () => {
// Select 'bulk' and test that all options are still available.
const checkbox = queryByInputItemName('bulk')
userEvent.click(checkbox)
await userEvent.click(checkbox)
await expectOptions(optionsStructural, optionsStructural)
})
})
......@@ -132,7 +132,7 @@ describe('', () => {
// Select PBE exchange and test that only PBE correlation is shown
// afterwards
const checkbox = queryByInputItemName('GGA_C_PBE_SOL')
userEvent.click(checkbox)
await userEvent.click(checkbox)
await expectOptions(['GGA_C_PBE_SOL', 'GGA_X_PBE_SOL'], optionsXC, false)
})
})
......@@ -160,7 +160,7 @@ describe('', () => {
// Start typing a value, it is expected that a list of suggestions will be
// shown shortly afterwards
userEvent.type(input, 'SOL')
await userEvent.type(input, 'SOL')
const suggestions = ['GGA_C_PBE_SOL', 'GGA_X_PBE_SOL']
for (const suggestion of suggestions) {
await screen.findByMenuItem(suggestion)
......@@ -168,7 +168,7 @@ describe('', () => {
// Select a suggested option by clicking it. This should update the list.
const suggestion = screen.getByMenuItem('GGA_C_PBE_SOL')
userEvent.click(suggestion)
await userEvent.click(suggestion)
await expectOptions(suggestions, suggestions, false)
})
})
......
......@@ -30,16 +30,10 @@ import {
import PropTypes from 'prop-types'
import clsx from 'clsx'
import { useRecoilValue } from 'recoil'
import MUIAddIcon from '@material-ui/icons/Add'
import AddCircleOutlineIcon from '@material-ui/icons/AddCircleOutline'
import CloseIcon from '@material-ui/icons/Close'
import HighlightOffIcon from '@material-ui/icons/HighlightOff'
import MoreVertIcon from '@material-ui/icons/MoreVert'
import AddCircleIcon from '@material-ui/icons/AddCircle'
import CancelIcon from '@material-ui/icons/Cancel'
import InputTitle from './InputTitle'
import { useSearchContext } from '../SearchContext'
import { Actions, ActionHeader, Action } from '../../Actions'
import StatisticsToggle from '../statistics/StatisticsToggle'
import { scales } from '../../plotting/common'
import { guiState } from '../../GUIMenu'
import { useBoolState } from '../../../hooks'
......@@ -85,17 +79,12 @@ const InputHeader = React.memo(({
anchored,
classes
}) => {
const { useStatisticState } = useSearchContext()
const styles = useStyles({classes: classes})
const [statistic, setStatistic] = useStatisticState(quantity)
const [anchorEl, setAnchorEl] = React.useState(null)
const isSettingsOpen = Boolean(anchorEl)
const [isStatsTooltipOpen, openStatsTooltip, closeStatsTooltip] = useBoolState(false)
const [isDragging, setDragging, setNotDragging] = useBoolState(false)
const [isTooltipOpen, openTooltip, closeTooltip] = useBoolState(false)
const align = useRecoilValue(guiState('align'))
const icon = useRecoilValue(guiState('icon'))
const iconSize = useRecoilValue(guiState('iconSize'))
const menu = useRecoilValue(guiState('menu'))
const openMenu = useCallback((event) => {
......@@ -117,26 +106,6 @@ const InputHeader = React.memo(({
onOpen: () => !isDragging && openTooltip()
}), [isTooltipOpen, closeTooltip, isDragging, openTooltip])
let RemoveIcon, AddIcon
if (icon === 'plain') {
RemoveIcon = CloseIcon
AddIcon = MUIAddIcon
} else if (icon === 'outlined') {
RemoveIcon = HighlightOffIcon
AddIcon = AddCircleOutlineIcon
} else if (icon === 'filled') {
RemoveIcon = CancelIcon
AddIcon = AddCircleIcon
}
const control = <Action
tooltip={statistic ? 'Remove statistics from the results panel.' : 'Display statistics for this filter in the results panel.'}
disabled={disableAnchoring}
onClick={() => setStatistic(old => !old)}
>
{statistic ? <RemoveIcon fontSize={iconSize}/> : <AddIcon fontSize={iconSize}/>}
</Action>
// Dedice the menu component. The tooltip of scale dropdown needs to be
// controlled, otherwise it won't close as we open the select menu.
const menuComp = menu === 'hidden'
......@@ -187,11 +156,10 @@ const InputHeader = React.memo(({
</Action>
return <Actions className={clsx(styles.root, className)}>
{align === 'left' && !disableAnchoring && control}
<ActionHeader disableSpacer>
<div
className={clsx(styles.row, anchored ? 'dragHandle' : undefined)}
style={{cursor: anchored ? (isDragging ? 'grabbing' : 'grab') : undefined}}
style={{cursor: anchored ? 'grabbing' : 'unset'}}
onMouseDown={handleMouseDown}
onMouseUp={handleMouseUp}
>
......@@ -207,7 +175,7 @@ const InputHeader = React.memo(({
</ActionHeader>
{actionsAlign === 'left' && actions}
{!disableStatistics && menuComp}
{align === 'right' && !disableAnchoring && control}
{!disableAnchoring && <StatisticsToggle quantity={quantity} disabled={disableAnchoring} />}
{actionsAlign === 'right' && actions}
</Actions>
})
......
......@@ -22,5 +22,5 @@ import InputHeader from './InputHeader'
test('initial state is loaded correctly', async () => {
const quantity = 'results.material.n_elements'