[OGUI-1853] Move actionables of objectTree in the table header #3247
Conversation
graduta
left a comment
graduta
left a comment
There was a problem hiding this comment.
Right now, the actionable buttons/search have not been moved to the table header, but instead at the top of the table.
Could you have a look at some ag-grid tables which offer the search input integrating in the header to provide a better UX? Somewhere under the name but in the headear of the table
While the collapse all can also be in the header most right
…ove-tree-actionables-to-table-header
…ove-tree-actionables-to-table-header
graduta
left a comment
graduta
left a comment
There was a problem hiding this comment.
Implementation wise, it works very well and the virtual table is once again not blocking the page.
Good job!
I left a few comments with regards to design and documentation
| * @readonly | ||
| */ | ||
| export const SortDirectionsEnum = Object.freeze({ | ||
| NONE: 0, |
There was a problem hiding this comment.
Is there a way to actually not have them sorted? I did not manage to get the UI in such a state.
Perhaps we should remove the option if that is the case?
| 'button.btn.sort-button', | ||
| { | ||
| onclick: () => onclick(label, nextSortOrder, hoverIcon), | ||
| title: `Sort by ${directionLabel}`, |
There was a problem hiding this comment.
Perhaps you meant: Sort ${directionLabel} by Name?
| const objectsToDisplay = objectsLoaded.filter((qcObject) => | ||
| qcObject.name.toLowerCase().includes(searchInput.toLowerCase())); | ||
| return virtualTable(model, 'main', objectsToDisplay); | ||
| return h('.scroll-y.flex-column.flex-grow', [ |
There was a problem hiding this comment.
Is there a need for scroll-y on this? The table header should remain on top rather than being part of the scroll
| h('tbody', [treeRows(model)]), | ||
| h('table.table.table-sm.text-no-select', h('tbody', [treeRows(model)])); | ||
|
|
||
| const tableHeaderRow = (model) => h('.bg-gray-light.pv2', [ |
There was a problem hiding this comment.
This method does not need the entire root model.
I think it is better to send only the object one.
Moreover the method is missing documentation.
|
|
||
| const tableHeaderRow = (model) => h('.bg-gray-light.pv2', [ | ||
| sortableTableHead({ | ||
| order: model.object.sortBy.order, |
There was a problem hiding this comment.
I think it would be nice to destruct the sortBy JSON before creating the node (same method tableHeaderRow but with a return statement returning the node rather than node being returned by arrow function) and assign default values to order and icon
| order: model.object.sortBy.order, | ||
| icon: model.object.sortBy.icon, | ||
| label: 'Name', | ||
| sortOptions: [SortDirectionsEnum.ASC, SortDirectionsEnum.DESC], |
There was a problem hiding this comment.
Did you mean to add "None" here as well?
| h('tbody', [treeRows(model)]), | ||
| h('table.table.table-sm.text-no-select', h('tbody', [treeRows(model)])); | ||
|
|
||
| const tableHeaderRow = (model) => h('.bg-gray-light.pv2', [ |
There was a problem hiding this comment.
I think the naming of these two methods is not clear enough.
tableHeaderRow is actually not a single row, right? But instead contains the Name of column header and another row with search input
On the other hand tableHeader is kind of a row but not a table and not a header but instead a div.
Try to come up with better descriptive namings
There was a problem hiding this comment.
All the newly added methods in this file are missing documentation. Please make sure to add
|
|
||
| const directionLabel = Object.keys(SortDirectionsEnum).find((key) => SortDirectionsEnum[key] === nextSortOrder); | ||
|
|
||
| return h( |
There was a problem hiding this comment.
I think it would be good to keep the same UX/UI for the users as for other applications that we develop.
Please have a look at Bookkeeping on what design they use and try to implement a similar one: attaching here a screenshot but perhaps running BKP locally would be more helpful
I have JIRA issue created
As a user, it is difficult to see that object tree can be filtered by name. Thus, the search input text (from top right in header) should be moved in the table header in a nice UI and UX manner.
Moreover, the (collapse all tree button) should be moved also in the table header(right most).
Moreover, the sort by name buton should also become part of the header so that when the user clicks on the header Name column, to display a sort icon up, or down or nothing depending on what is currently being sorted.
See for example how Bookkeeping sorts by Title on page "Log Entries" (log-overview)