Horreum UI refactoring

[johnaohara]

Horreum UI refactoring.

In summary, here the most important changes:

• Main toolbar refactored as left-sidebar
• Test-specific toolbar improved
  • Runs and datasets showed as inner tabs
  • Changes and reports are no more standalone pages, they are under a specific test
  • "General" tab renamed as "Settings"
• Folders navigation refactored using breadcrumb

[willr3]

Overall I like the idea of moving items under the associated Test and preserving the selected test when navigating between them.

My opinion on changes:

• The login button should stay right aligned in the top nav.
• Only 2 (3 with Admin) options in the sidebar uses a lot of vertical space.
• What happened to Runs? Users upload Runs not Datasets so I think Datasets are what should go away, not Runs.

[johnaohara]

The login button should stay right aligned in the top

That was my intention, it is layout issue that needs resolving

Only 2 (3 with Admin) options in the sidebar uses a lot of vertical space.

I was thinking about being about to "favourite" tests which would appear under the "Test" menu, this would fill this sidebar up.

It is also possible to collapse the sidebar so you get the full width of the screen by pressing the button

The other option is to put Tests / Schema / Admin back on the top menu and some of the

What happened to Runs? Users upload Runs not Datasets so I think Datasets are what should go away, not Runs.

I was experimenting with simplifying the UI, atm the core abstraction for change detection is dataset. The "Runs" View allows you to compare runs and delete them, but is similar to datasets.

Overall we need to think about what is the best way to visualize the Run/Dataset abstraction, or if we need that abstraction at all

[johnaohara]

The "Folder" navigation could move into the navigation bar

and we get a lot of space back at the top of the main view

instead of what we have right now:

[lampajr]

What I did

In the previous commits I have fixed (hopefully) some UI issues that I'll summarize here:

1. Aligned the header toolbar to the right such that the Log in is in the top-right corner
   
2. Simplified the changes tab query params, removing the weird http://localhost:8080/test/113/?test=[object%20Object] test param (which is not really needed as already present in the URI)
   
3. Fixed the Alert box message as it was overflowing when the left sidebar was opened
4. Fixed the reports navigation as it was commented out
   • Here I re-established the same UI that was there before, I simply changed the full URI to contain the test/:testId as root.
   • For instance, I changed from /reports/table/config/:configId to /test/:testId/reports/table/config/:configId
   • Now the reports link moves the the user back to the reports tab for that specifc test
     

Open points

In conclusion I think that from this pull request point of view we have still ~4 open points:

• Is the reports navigation I re-established fine? Or were you thinking about a different approach?
  • Maybe this is a question more for @johnaohara
• Right now the folder selection is just a dropdown list
  
  • Once selected the folder name is not shown in that dropdown button
    
  • Do we want to implement the folder structure as mentioned/showed in this comment Horreum UI refactoring #1290 (comment) ?
• There some portions of the code that have been commented out, I think more for testing/experimentations so I think that if we agree on these new changes we can get rid of them (as well as for some no longer needed imports)
• Missing Horreum logo

[lampajr]

Alternatively another option for the Folder structure could be to use the breadcrumb approach like:

I still don't know where the Team Selector could be placed, as I don't really like it next to the breadcrumb.
Any opinion on this approach?

[lampajr]

Do we need this? the test id is provided by props therefore it should not change. In fact the function setSelectedTabs is not called anywhere

[johnaohara]

I think we can remove this now

[lampajr]

As discussed added additional commit (ca3fff3) to add folders navigation using breadcrumb:

I tried to keep the folder structure, i.e., Horreum as root of the tree and then all the folders, an alternative could be to remove that root layer:

[lampajr]

For better navigation the last commit introduces the original folder where the test resides, e.g.,

[lampajr]

Since we changed some of the URL paths, e.g., for changes, IIUC we should change the URLs we inject into the email notification as well (assuring they will point to the correct resources)

[lampajr]

Since we changed some of the URL paths, e.g., for changes, IIUC we should change the URLs we inject into the email notification as well (assuring they will point to the correct resources)

Updated ✔️ hopefully I did not miss anything else.

IMO this PR could be considered ready for review but I'd left your final comment here @johnaohara @stalep.

[johnaohara]

I tried to keep the folder structure, i.e., Horreum as root of the tree and then all the folders, an alternative could be to remove that root layer:

I think the drop down with "24" is confusing, it is not obvious what the "24" refers to. I think using "Test" in the breadcrumb for the dropdown might be more intuitive for users;

I would also drop the "<" arrow from Horreum, that way it appears to be a tree, with "Horreum" as the root

[johnaohara]

When I click on a "Run" of a "Dataset", the breadcrumb disappears.

I think we should retain the breadcrumb on all views

[johnaohara]

Is the reports navigation I re-established fine? Or were you thinking about a different approach?

* Maybe this is a question more for @johnaohara

@lampajr What changes were made to the reports navigation? I can not see an issue, but was wondering what had changed?

[lampajr]

Is the reports navigation I re-established fine? Or were you thinking about a different approach?

* Maybe this is a question more for @johnaohara

@lampajr What changes were made to the reports navigation? I can not see an issue, but was wondering what had changed?

@johnaohara actually nothing changed (it was simply commented out, so I re-established it), the only difference with respect to the as-is is the URL that hanged from /reports/table/config/:configId to /test/:testId/reports/table/config/:configId

I think the drop down with "24" is confusing, it is not obvious what the "24" refers to. I think using "Test" in the breadcrumb for the dropdown might be more intuitive for users;

I agree, much cleaner and easier to underastand. Working on it.

When I click on a "Run" of a "Dataset", the breadcrumb disappears.
I think we should retain the breadcrumb on all views

Oh, good catch! I did not notice. Working on this as well.

[lampajr]

I was able to keep the breadcrumb on run and dataset view but unfortunately we don't have information about the folder (this could require some refactoring to pass that information down till the final view).

This is the actual result

Removed the number of folders from the dropdown, this is the result as you already showed:

[johnaohara]

On the Tests view, there is the option to filter tests by ownership in a dropdown.

That same dropdown is in the Schema view, but is not currently functional. I think we should get this wired up

[lampajr]

Hi @johnaohara

I addressed the other minors, especially I re-instated the setLoading on AllTests page.

That same dropdown is in the Schema view, but is not currently functional. I think we should get this wired up

Yeah that's something we should do, I checked and it seems this is not supported from the backend therefore it is not about UI changes only. Do you want to do that as part of this PR? Or do you prefer to create a separate issue to manage both backend and frontend?

[johnaohara]

Hi @johnaohara

I addressed the other minors, especially I re-instated the setLoading on AllTests page.

That same dropdown is in the Schema view, but is not currently functional. I think we should get this wired up

Yeah that's something we should do, I checked and it seems this is not supported from the backend therefore it is not about UI changes only. Do you want to do that as part of this PR? Or do you prefer to create a separate issue to manage both backend and frontend?

If it is not supported in the backend, we should hide the component in the UI and add that in a separate PR

[lampajr]

If it is not supported in the backend, we should hide the component in the UI and add that in a separate PR

I agree, hidden in this pr and created #1589 for its implementation.
I raised that as bug since it is actually showed in production so users would exepct to work.

[johnaohara]

@lampajr please can you change title the PR, squash commits and remove from Draft status?

[shivarm]

@johnaohara will we nee all commentend code, I am asking because its kind of mesh.

[johnaohara]

@johnaohara will we nee all commentend code, I am asking because its kind of mesh.

Not in this instance. In this PR, the UI is being re-organised as an interim step, but there is more re factoring to be performed to allow for Unit testing etc. As we perform work to improve the UI components, rather than just moving them around, I think we want more details about the components then

[whitingjr]

Introducing 3 clicks to navigation is a backward step imo.

1. click menu to open
2. click Test or Schema
3. click menu to hide
   Created Make main menu mouse hover sensitive. #1622 for a feature request to make it use mouse events to open/close.