[OGUI-448] & [OGUI-453] Fix small visual bugs (#504)

* [OGUI-448] Fix small visual bugs on sidebar layouts

* [OGUI-448] Add back gray spinner

* [OGUI-448] Improve loading objects UI experience

* [OGUI-448] Fix test

* [OGUI-448] Add loading icon on sidetree

* [OGUI-448] Fix undefiend error message

* [OGUI-453] Trigger resize so that virtual node change is triggerred as well

* [OGUI-448] Add button inline with title

* [OGUI-448] Fix error message out of plot

* [OGUI-448] Remove duplicated error message

* Bump patch version of qcg

QualityControl/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@aliceo2/qc",
-  "version": "1.7.0",
+  "version": "1.7.1",
   "description": "O2 Quality Control Web User Interface",
   "author": "Vladimir Kosmala",
   "contributors": [

QualityControl/public/Model.js

@@ -106,15 +106,12 @@ export default class Model extends Observable {
   handleLocationChange() {
     switch (this.router.params.page) {
       case 'layoutList':
-        this.layout.loadList()
-          .then(() => {
-            this.page = 'layoutList';
-            this.notify();
-          }).catch(() => true); // error is handled inside loadList
+        this.page = 'layoutList';
+        this.layout.loadList();
         break;
       case 'layoutShow':
         if (!this.router.params.layoutId) {
-          // TODO: notification(`Argument layoutId is URL is missing`);
+          this.notification.show(`Argument layoutId in URL is missing`, 'warning', 2000);
           this.router.go('?', true);
           return;
         }

QualityControl/public/app.css

@@ -38,4 +38,8 @@
 select {-webkit-appearance: none;-moz-appearance: none;text-indent: 1px;text-overflow: '';}
 .select-tab {border: 0;border-bottom: 2px solid var(--color-blue);border-radius: 0;margin-bottom: -2px;color: var(--color-blue);}
 
-.checker-label{font-weight: bold; margin-bottom: 0; color: var(--color-gray-dark) }
+.checker-label{font-weight: bold; margin-bottom: 0; color: var(--color-gray-dark) }
+
+.actionable-icon {cursor: pointer;}
+.actionable-icon:hover {transform: scale(1.2);}
+.actionable-icon:active {transform: scale(1.5);}

QualityControl/public/common/header.js

@@ -48,7 +48,6 @@ const commonHeader = (model) => h('.flex-grow', [
   ' ',
   h('span.f4.gray', 'Quality Control'),
   model.loader.active && h('span.f4.mh1.gray', spinner())
-  // TODO To be redesigned. It is not visible in case of error and UI is hanging
 ]);
 
 /**

QualityControl/public/common/sidebar.js

@@ -24,9 +24,7 @@ export default function sidebar(model) {
   // Spacial case when sidebar is used as a required form or perperty editor
   if (model.router.params.page === 'layoutShow' && model.layout.editEnabled) {
     return h('nav.sidebar.sidebar-extend', {class: ''}, [
-      h('.sidebar-content', [
-        objectTreeSidebar(model)
-      ])
+      h('.sidebar-content', [objectTreeSidebar(model)])
     ]);
   }
 
@@ -44,8 +42,7 @@ export default function sidebar(model) {
 const sidebarMenu = (model) => [
   exploreMenu(model),
   myLayoutsMenu(model),
-  h('.menu-title', ''),
-  refreshOptions(model),
+  model.object.isOnlineModeEnabled ? refreshOptions(model) : h('.menu-title', {style: 'flex-grow:1'}, ''),
   statusMenu(model),
   collapseSidebarMenuItem(model)
 ];
@@ -83,20 +80,21 @@ const exploreMenu = (model) => [
  * @return {vnode}
  */
 const myLayoutsMenu = (model) => [
-  h('.menu-title', model.sidebar ? 'My Layouts' : ''),
+  h('.menu-title.flex-row', model.sidebar ? [
+    h('', {style: 'width: 90%'}, 'My Layouts'),
+    h('.ph2.text-right.actionable-icon', {
+      title: 'Create a new layout',
+      onclick: () => model.layout.newItem(prompt('Choose a name for the new layout:'))
+    }, iconPlus())
+  ] : ''),
   model.layout.myList.match({
     NotAsked: () => null,
     Loading: () => h('.menu-item', 'Loading...'),
-    Success: (list) => list.map((layout) => myLayoutsMenuItem(model, layout)),
+    Success: (list) => h('.scroll-y', {
+      style: 'min-height: 10em;'
+    }, list.map((layout) => myLayoutsMenuItem(model, layout))),
     Failure: (error) => h('.menu-item', error),
   }),
-  h('a.menu-item', {
-    title: 'New layout...',
-    style: 'display:flex',
-    onclick: () => model.layout.newItem(prompt('Choose a name of the new layout:'))
-  }, [
-    h('span', iconPlus()), model.sidebar && itemMenuText('New layout...')
-  ])
 ];
 
 /**
@@ -140,7 +138,7 @@ const myLayoutsMenuItem = (model, layout) => h('a.menu-item.w-wrapped', {
 const refreshOptions = (model) => [
   h('', {
     class: model.sidebar ? 'menu-title' : '',
-    style: model.object.isOnlineModeEnabled ? 'flex-grow:1' : 'visibility: hidden; flex-grow:1'
+    style: 'flex-grow:1; height:auto'
   }, [
     model.sidebar &&
     [

QualityControl/public/layout/Layout.js

@@ -26,7 +26,6 @@ export default class Layout extends Observable {
     this.requestedLayout = RemoteData.notAsked();
 
     this.searchInput = '';
-    this.searchResult = null; // null means no search, sub-array of `list`
 
     this.editEnabled = false; // activate UI for adding, dragging and deleting tabObjects inside the current tab
     this.editingTabObject = null; // pointer to a tabObject being modified
@@ -288,8 +287,7 @@ export default class Layout extends Observable {
   }
 
   /**
-   * Set uset input for search and use a fuzzy algo to filter list of layouts.
-   * Result is set inside `searchResult`.
+   * 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
    */
@@ -312,6 +310,8 @@ export default class Layout extends Observable {
     this.editEnabled = true;
     this.editOriginalClone = JSON.parse(JSON.stringify(this.item)); // deep clone
     this.editingTabObject = null;
+    window.dispatchEvent(new Event('resize'));
+
     this.notify();
   }
 

QualityControl/public/layout/layoutListHeader.js

@@ -1,5 +1,6 @@
 import {h} from '/js/src/index.js';
 
+
 /**
  * Shows header of list of layouts with one search input to filter them
  * @param {Object} model
@@ -26,6 +27,4 @@ export default (model) => [
  * @param {Object} model
  * @return {vnode}
  */
-const howManyItems = (model) => model.layout.searchResult
-  ? `${model.layout.searchResult.length} found of ${model.layout.list.length}`
-  : `${model.layout.list && model.layout.list.length} items`;
+const howManyItems = (model) => `${model.layout.list && model.layout.list.length} items`;

QualityControl/public/loader/spinner.js

@@ -1,6 +1,8 @@
 import {h} from '/js/src/index.js';
 
-export default () => h('.atom-spinner',
+export default (size) => h('span.pageLoading', {
+  style: size ? `font-size: ${size}em`: '',
+}, h('.atom-spinner',
   h('.spinner-inner',
     [
       h('.spinner-line'),
@@ -11,4 +13,4 @@ export default () => h('.atom-spinner',
       )
     ]
   )
-);
+));

QualityControl/public/object/QCObject.js

@@ -20,6 +20,7 @@ export default class QCObject extends Observable {
     this.currentList = [];
     this.list = null;
 
+    this.objectsRemote = RemoteData.notAsked();
     this.selected = null; // object - id of object
     this.selectedOpen = false;
     this.objects = {}; // objectName -> RemoteData
@@ -189,13 +190,17 @@ export default class QCObject extends Observable {
    */
   async loadList() {
     if (!this.isOnlineModeEnabled) {
+      this.objectsRemote = RemoteData.loading();
+      this.notify();
       this.queryingObjects = true;
       let offlineObjects = [];
       const result = await this.qcObjectService.getObjects();
       if (result.isSuccess()) {
         offlineObjects = result.payload;
       } else {
-        this.model.notification.show(`Failed to retrieve list of objects due to ${result.message}`, 'danger', Infinity);
+        const errorMessage = result.payload.message ? result.payload.message : result.payload;
+        const failureMessage = `Failed to retrieve list of objects due to ${errorMessage}`;
+        this.model.notification.show(failureMessage, 'danger', Infinity);
       }
       this.list = offlineObjects;
 
@@ -219,6 +224,7 @@ export default class QCObject extends Observable {
         this.selected = this.list.find((object) => object.name === this.selected.name);
       }
       this.queryingObjects = false;
+      this.objectsRemote = RemoteData.success();
       this.notify();
     } else {
       this.loadOnlineList();
@@ -241,6 +247,8 @@ export default class QCObject extends Observable {
    * Ask server for online objects and fills tree with them
    */
   async loadOnlineList() {
+    this.objectsRemote = RemoteData.loading();
+    this.notify();
     let onlineObjects = [];
     const result = await this.qcObjectService.getOnlineObjects();
     if (result.isSuccess()) {
@@ -254,7 +262,8 @@ export default class QCObject extends Observable {
         open: false
       };
     } else {
-      const failureMessage = `Failed to retrieve list of online objects due to ${result.message}`;
+      const errorMessage = result.payload.message ? result.payload.message : result.payload;
+      const failureMessage = `Failed to retrieve list of online objects due to ${errorMessage}`;
       this.model.notification.show(failureMessage, 'danger', Infinity);
     }
 
@@ -264,6 +273,8 @@ export default class QCObject extends Observable {
     this.listOnline = onlineObjects;
     this.currentList = onlineObjects;
     this.search('');
+    this.objectsRemote = RemoteData.success();
+    this.notify();
   }
 
   /**
@@ -344,19 +355,22 @@ export default class QCObject extends Observable {
    * @param {Array.<string>} objectsName - e.g. /FULL/OBJECT/PATH
    */
   async loadObjects(objectsName) {
+    this.objectsRemote = RemoteData.loading();
+    this.notify();
     if (!objectsName || !objectsName.length) {
       return;
     }
 
-    const result = await this.qcObjectService.getObjectsByName(objectsName);
-    if (!result.isSuccess()) {
+    this.objectsRemote = await this.qcObjectService.getObjectsByName(objectsName);
+    this.notify();
+    if (!this.objectsRemote.isSuccess()) {
       // it should be always status=200 for this request
       this.model.notification.show('Failed to refresh plots when contacting server', 'danger', Infinity);
       return;
     }
 
     // eslint-disable-next-line
-    const objects = JSROOT.JSONR_unref(result.payload);
+    const objects = JSROOT.JSONR_unref(this.objectsRemote.payload);
     for (const name in objects) {
       if (objects[name].error) {
         this.objects[name] = RemoteData.failure(objects[name].error);

QualityControl/public/object/objectDraw.js

@@ -116,9 +116,9 @@ export function draw(model, tabObject, options, location = '') {
       h('.animate-slow-appearance', 'Loading')
     ]);
   } else if (objectRemoteData.isFailure()) {
-    content = h('.absolute-fill.flex-column.items-center.justify-center', [
-      h('.p4.f6', objectRemoteData.payload),
-    ]);
+    content = h('.scroll-y.absolute-fill.p1.f6', {
+      style: 'word-break: break-all;'
+    }, objectRemoteData.payload);
   } else {
     if (model.object.isObjectChecker(objectRemoteData.payload)) {
       return checkersPanel(objectRemoteData.payload, location);

QualityControl/public/object/objectTreeHeader.js

@@ -21,7 +21,7 @@ export default function objectTreeHeader(model) {
     h('.w-33.text-center', [
       h('b.f4', 'Objects'),
       ' ',
-      h('span', `(${howMany})`),
+      model.object.objectsRemote.isSuccess() && h('span', `(${howMany})`),
     ]),
     h('.flex-grow.text-right', [
       h('.dropdown', {

QualityControl/public/object/objectTreePage.js

@@ -1,4 +1,5 @@
 import {h, iconBarChart, iconCaretRight, iconResizeBoth, iconCaretBottom} from '/js/src/index.js';
+import spinner from '../loader/spinner.js';
 import {draw} from './objectDraw.js';
 import infoButton from './../common/infoButton.js';
 
@@ -10,7 +11,15 @@ import infoButton from './../common/infoButton.js';
  */
 export default (model) => h('.flex-column.absolute-fill', {key: model.router.params.page}, [
   h('.flex-row.flex-grow', [
-    h('.flex-grow.scroll-y', tableShow(model)),
+    h('.flex-grow.scroll-y',
+      model.object.objectsRemote.match({
+        NotAsked: () => null,
+        Loading: () => h('.absolute-fill.flex-column.items-center.justify-center.f5', [
+          spinner(5), h('', 'Loading Objects')
+        ]),
+        Success: () => tableShow(model),
+        Failure: () => null, // notification is displayed
+      })),
     h('.animate-width.scroll-y',
       {
         style: {

QualityControl/public/object/objectTreeSidebar.js

@@ -1,4 +1,5 @@
 import {h} from '/js/src/index.js';
+import spinner from '../loader/spinner.js';
 import {draw} from './objectDraw.js';
 import {iconCaretBottom, iconCaretRight, iconBarChart, iconResizeBoth} from '/js/src/icons.js';
 
@@ -12,7 +13,16 @@ import {iconCaretBottom, iconCaretRight, iconBarChart, iconResizeBoth} from '/js
  */
 export default (model) => h('.flex-column.h-100', [
   h('.m2.mv3', searchForm(model)),
-  h('.h-100.scroll-y', treeTable(model)),
+  h('.h-100.scroll-y',
+    model.object.objectsRemote.match({
+      NotAsked: () => null,
+      Loading: () => h('.flex-column.items-center.justify-center.f5', [
+        spinner(3), h('', 'Loading Objects')
+      ]),
+      Success: () => treeTable(model),
+      Failure: () => null, // notification is displayed
+    }),
+  ),
   objectPreview(model)
 ]);
 
@@ -113,7 +123,7 @@ const treeRows = (model) => !model.object.sideTree
  * @return {vnode}
  */
 function searchRows(model) {
-  return !model.object.searchResult ? null : model.object.searchResult.map((item)=> {
+  return !model.object.searchResult ? null : model.object.searchResult.map((item) => {
     const path = item.name;
     const className = item && item === model.object.selected ? 'table-primary' : '';
 

QualityControl/test/public/mocha-index.js

@@ -135,7 +135,7 @@ describe('QCG', function() {
     });
 
     it('should have selected layout in the sidebar highlighted', async () => {
-      const layoutClassList = await page.evaluate(() => document.querySelector('body > div > div > nav > a:nth-child(5)').classList);
+      const layoutClassList = await page.evaluate(() => document.querySelector('body > div > div > nav > div:nth-child(5) > a:nth-child(1)').classList);
       assert.deepStrictEqual(layoutClassList, {0: 'menu-item', 1: 'w-wrapped', 2: 'selected'});
     });