-
Notifications
You must be signed in to change notification settings - Fork 14
[OGUI-1760] Update search UI for object path and add filters #3065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 19 commits
b43546a
37df26e
bbb5b99
f539c53
fd99dac
d74068e
98eecce
1606cc9
fb43fe6
bff7808
710cb66
84892d7
9e762f8
a054459
768ad12
f8ce94b
b34779b
c5cd6e8
7ff33cf
1df126d
02965cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,12 +47,12 @@ | |
/** | ||
* Shows the page specific header (center and right side) | ||
* @param {Model} model - root model of the application | ||
* @returns {{centerCol: vnode, rightCol: vnode} | null} | ||
Check warning on line 50 in QualityControl/public/common/header.js
|
||
*/ | ||
const headerSpecific = (model) => { | ||
const { layoutListModel, filterModel, layout, object, page } = model; | ||
const { filterModel, layout, object, page } = model; | ||
switch (page) { | ||
case 'layoutList': return LayoutListHeader(layoutListModel); | ||
case 'layoutList': return LayoutListHeader(); | ||
case 'layoutShow': return layoutViewHeader(layout, filterModel); | ||
case 'objectTree': return objectTreeHeader(object, filterModel); | ||
case 'objectView': return objectViewHeader(model); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
/** | ||
* @license | ||
* Copyright 2019-2020 CERN and copyright holders of ALICE O2. | ||
* See http://alice-o2.web.cern.ch/copyright for details of the copyright holders. | ||
* All rights not expressly granted are reserved. | ||
* | ||
* This software is distributed under the terms of the GNU General Public | ||
* License v3 (GPL Version 3), copied verbatim in the file "COPYING". | ||
* | ||
* In applying this license CERN does not waive the privileges and immunities | ||
* granted to it by virtue of its status as an Intergovernmental Organization | ||
* or submit itself to any jurisdiction. | ||
*/ | ||
|
||
/** | ||
* @typedef {object} Filter | ||
* @property {string} key - searchable key of the filter. | ||
* @property {function(): (string)} friendlyName - friendly name of the filter else key. | ||
* @property {function(): (string)} inputPlaceholder - Input element's placeholder, use examples of filter. | ||
* @property {function(): (boolean)} isActive - has the filter any active value('s) | ||
* @property {function(): (string|string[]|null)} getValue - gets the current value('s) of the filter | ||
* @property {function(string): void} set - set value of the filter | ||
* @property {Function} reset - reset filter to default state | ||
*/ | ||
|
||
/** | ||
* creates a key-value filter. | ||
* @param {string} key - key used to save and retrieve value. | ||
* @param {string|null} friendlyName - friendly name of the filter. | ||
* @param {string|null} inputPlaceholder - input placeholder text. | ||
* @param {string} value - value associated with key. | ||
* @returns {Filter} key-value filter object. | ||
*/ | ||
export function createKeyValueFilter(key, friendlyName = null, inputPlaceholder = null, value = '') { | ||
return { | ||
key, | ||
friendlyName: () => friendlyName ? friendlyName : key, | ||
inputPlaceholder: () => inputPlaceholder ? inputPlaceholder : '', | ||
getValue: () => value ? value : null, | ||
// trim checks if value is a string value, test this | ||
isActive: () => Boolean(value && value.trim()), | ||
// eslint-disable-next-line @stylistic/js/brace-style | ||
set: (v) => { value = v; }, | ||
// eslint-disable-next-line @stylistic/js/brace-style | ||
reset: () => { value = ''; }, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,18 +15,10 @@ | |
import { h } from '/js/src/index.js'; | ||
|
||
/** | ||
* Shows header of list of layouts with one search input to filter them | ||
* @param {LayoutListModel} layoutListModel - The model handling the state of the LayoutListPage | ||
* @returns {{centerCol: vnode, rightCol: vnode}} - object with virtual node elements | ||
* Shows header of list of layouts. | ||
* @returns {vnode} - virtual node element | ||
*/ | ||
export default (layoutListModel) => ({ | ||
centerCol: h('.flex-grow.text-center', [h('b.f4', 'Layouts')]), | ||
rightCol: h('.w-33.text-right', [ | ||
h('input.form-control.form-inline.mh1.w-33', { | ||
placeholder: 'Search', | ||
type: 'text', | ||
value: layoutListModel.searchInput, | ||
oninput: (e) => layoutListModel.search(e.target.value), | ||
}), | ||
]), | ||
}); | ||
export default () => [ | ||
h('.w-50.text-center', [h('b.f4', 'Layouts')]), | ||
|
||
h('.flex-grow.text-right'), | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
/** | ||
* @license | ||
* Copyright 2019-2020 CERN and copyright holders of ALICE O2. | ||
* See http://alice-o2.web.cern.ch/copyright for details of the copyright holders. | ||
* All rights not expressly granted are reserved. | ||
* | ||
* This software is distributed under the terms of the GNU General Public | ||
* License v3 (GPL Version 3), copied verbatim in the file "COPYING". | ||
* | ||
* In applying this license CERN does not waive the privileges and immunities | ||
* granted to it by virtue of its status as an Intergovernmental Organization | ||
* or submit itself to any jurisdiction. | ||
*/ | ||
|
||
// Adopted from Bookkeeping/lib/public/components/Filters/common/filtersPanelPopover.js | ||
import { h, popover, PopoverAnchors, PopoverTriggerPreConfiguration } from '/js/src/index.js'; | ||
// import { tooltip } from '../../common/popover/tooltip.js'; | ||
graduta marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
/** | ||
* imports for JSDoc + VSCode navigation: | ||
* @import SearchFilterModel from './model/SearchFilterModel.js'; | ||
*/ | ||
|
||
/** | ||
* Return the filters panel popover trigger | ||
* @returns {Component} the button component | ||
*/ | ||
const filtersToggleTrigger = () => h('button#openFilterToggle.btn.btn.btn-primary', 'Filters'); | ||
|
||
/** | ||
* Create main header of the filters panel | ||
* @param {SearchFilterModel} searchFilterModel {@link SearchFilterModel} filtering model. | ||
* @returns {Component} main panel header. | ||
*/ | ||
const filtersToggleContentHeader = (searchFilterModel) => h('.flex-row.justify-between', [ | ||
h('.f4', 'Filters'), | ||
h( | ||
'button#reset-filters.btn.btn-danger', | ||
{ | ||
onclick: () => searchFilterModel.resetAll(), | ||
disabled: searchFilterModel.allInActive() ? true : false, | ||
}, | ||
'Reset all filters', | ||
), | ||
]); | ||
|
||
/** | ||
* Return the filters panel popover content section | ||
* @param {SearchFilterModel} searchFilterModel the searchFilter model | ||
* @returns {Component} the filters section | ||
*/ | ||
export const filtersSection = (searchFilterModel = {}) => [ | ||
searchFilterModel.getAll().flatMap((filter) => [ | ||
h('.flex-row.g2', [ | ||
h('.w-30.f5.flex-row.items-center.g2', filter.friendlyName()), | ||
h('.w-70', [ | ||
h('input.form-control.w-100', { | ||
placeholder: filter.inputPlaceholder(), | ||
type: 'text', | ||
value: filter.getValue(), | ||
onchange: (e) => searchFilterModel.setValue(filter.key, e.target.value), | ||
}), | ||
]), | ||
]), | ||
]), | ||
]; | ||
|
||
/** | ||
* Return the filters panel popover content (i.e. the actual filters) | ||
* @param {SearchFilterModel} searchFilterModel the filtering model | ||
* @returns {Component} the filters panel | ||
*/ | ||
const filtersToggleContent = (searchFilterModel) => h('.w-l.flex-column.p3.g3', [ | ||
filtersToggleContentHeader(searchFilterModel), | ||
filtersSection(searchFilterModel), | ||
]); | ||
|
||
/** | ||
* Return component composed of the filtering popover and its button trigger | ||
* @param {SearchFilterModel} searchFilterModel the filtering model | ||
* @returns {Component} the filter component | ||
*/ | ||
export const filtersPanelPopover = (searchFilterModel) => popover( | ||
filtersToggleTrigger(), | ||
filtersToggleContent(searchFilterModel), | ||
{ | ||
...PopoverTriggerPreConfiguration.click, | ||
anchor: PopoverAnchors.RIGHT_START, | ||
}, | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ import FolderModel, { FolderType } from '../../../folder/model/FolderModel.js'; | |
import LayoutCardModel from './LayoutCardModel.js'; | ||
import { BaseViewModel } from '../../../common/abstracts/BaseViewModel.js'; | ||
import { RequestFields } from '../../../common/RequestFields.enum.js'; | ||
import SearchFilterModel from './SearchFilterModel.js'; | ||
|
||
/** | ||
* LayoutListModel namespace to control the layoutCards spread between its folders | ||
|
@@ -28,8 +29,15 @@ export default class LayoutListModel extends BaseViewModel { | |
constructor(model) { | ||
super(); | ||
this.model = model; | ||
this._searchInput = ''; | ||
this.folders = new Map(); | ||
this.searchFilterModel = new SearchFilterModel(); | ||
this.searchFilterModel.observe(() => { | ||
if (!this.searchFilterModel.allInActive()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the filter should only send the active filters and not all of them. For example if we have 2 parameters, objectPath and tab, we only want to attach one of them to the request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moreover, I think here the if condition should as well use the getActive like recommended in the other comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. getAllAsObject refactored into getAllActiveAsObject. Beheviour is as expected now with multiple filters or multiple registered with only one active. |
||
this.search(undefined, this.searchFilterModel.getAllAsObject()); | ||
} else { | ||
this.search(undefined, undefined); | ||
} | ||
}); | ||
|
||
this._initializeFolders(); | ||
} | ||
|
@@ -64,27 +72,35 @@ export default class LayoutListModel extends BaseViewModel { | |
* @returns {string} The trimmed search input | ||
*/ | ||
get searchInput() { | ||
return this._searchInput.trim(); | ||
return this.searchFilterModel.searchInput.trim(); | ||
} | ||
|
||
set searchInput(value) { | ||
this.searchFilterModel.searchInput = value; | ||
} | ||
|
||
/** | ||
* Set user's input for search and use a fuzzy algo to filter list of layouts. | ||
* Fuzzy allows missing chars "aaa" can find "a/a/a" or "aa/a/bbbbb" | ||
* @param {string} searchInput - string input from the user to search by | ||
* @param {string} objectPath - string input from the user to search layouts by objectPath | ||
* @returns {undefined} | ||
* Fuzzy allows missing chars "aaa" can find "a/a/a" or "aa/a/bbbbb". | ||
* If searchInput and objectPath are not included get all non-filtered layouts. | ||
* All params can be undefined if you want all layouts. | ||
* @param {string} searchInput - string input from the user to search by. | ||
* @param {object} filters - filters object contains all filter key value pairs in one object. | ||
*/ | ||
search(searchInput, objectPath) { | ||
if (objectPath === undefined) { | ||
this._searchInput = searchInput; | ||
search(searchInput, filters) { | ||
if (searchInput === undefined && filters === undefined) { | ||
// Get all layouts | ||
this.model.services.layout.getLayouts(RequestFields.LAYOUT_CARD, undefined); | ||
} else if (filters === undefined) { | ||
// Normal offline search | ||
this.searchInput = searchInput; | ||
this.folders.forEach((folder) => { | ||
folder.searchInput = new RegExp(searchInput, 'i'); | ||
}); | ||
this.notify(); | ||
} else { | ||
const layoutService = this.model.services.layout; | ||
this._searchInput = objectPath; | ||
layoutService.getLayouts(RequestFields.LAYOUT_CARD, { objectPath }, this.model); | ||
// online search using filters | ||
this.model.services.layout.getLayouts(RequestFields.LAYOUT_CARD, filters); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not just disable an eslint if inconvenient.
I think in this case, you cannot just remove the
{}
because of the other eslint rule not allowing assignments in arrow functions.Thus, a cleaner solution is to apply the exception of one line function (contrary to the previous comments, I understand)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand it is actually the same as you initially added. It was an oversight from my side, forgetting about the assignment eslint rule, I apologise for that