SaltGUI icon indicating copy to clipboard operation
SaltGUI copied to clipboard

Open a row/link in a new tab

Open rcmoutinho opened this issue 1 year ago • 16 comments

Is your feature request related to a problem? Please describe.

Many pages like pillars, grains, jobs, etc., contain an extensive list that might take a while to load, depending on the number of minions. In this context, whenever you click in the row, it loads the content on the same page. But once you go back, it needs to load everything again.

There is a good side to always getting the latest data. But it is very annoying when you have a huge list or a filter to load, and when you get back to the page, it loads the default content again. It would be amazing to open a new tab for each job we want to look deeper into without the need to load the whole list for each of them (that would also decrease the load on the server calls).

Describe the solution you'd like

Create a configuration to force every row click to open in a new tab so the user can decide whether this behavior is more convenient. Right-clicking and opening a new tab could also be possible as a solution.

Describe alternatives you've considered

Adding an extra button on this kind of page to open the content in a new tab (if the row behavior is more challenging to implement).

Additional context

I could contribute if you gave me some guidance. I saw panels and page folders, but I'm not sure exactly how to add things there.

Thanks in advance for reading the message and maintaining this great project. Cheers!

rcmoutinho avatar Apr 25 '24 18:04 rcmoutinho

migrated to PR

erwindon avatar Apr 25 '24 19:04 erwindon

@rcmoutinho I've turned this issue into a PR so that I can experiment on a branch and let that be visible for you (and others). The first implementation tries to use the CTRL key to open the page in a new tab. We'll see how that works out...

erwindon avatar Apr 25 '24 19:04 erwindon

Outstanding! Sounds great to me! Looking forward to seeing your tests! Thanks again!

rcmoutinho avatar Apr 25 '24 19:04 rcmoutinho

I've pushed a small implementation on the branch from this PR.

  • ALT-click now opens a new page in a new browser-tab. I tried CTRL-click, but that interfered too much with the "select-text" function of the browser. I tried adding extra menu-items but that would mean lots of extra menu items, which is too much. I tried adding icon on the row, but that left the menu-item without solution.
  • For this branch this ONLY works for page "Grains" at this moment:
    • For ALT-click on the table-row; and
    • For ALT-click on the (only) menu item of the menu for each row.
  • The new page has no idea that it was opened this way. But there are several possible variations:
    • The new page does not have a logo/menubar/cmdbutton on top; and/or
    • The new page does not have a close-button on the page; and/or
    • The new page does not have a "last-7-jobs" panel; and/or
    • suggestions?...

erwindon avatar Apr 25 '24 20:04 erwindon

To be honest, whatever works to open a new tab, I would be happy.

And just tested locally, and the ALT-Click works great 🎉

My opinion on what's available on the new page is just what is expected from the click. I would not add a menu or last job panel, just the information about the click (grain, pillar, job, or whatever info we are clicking). It would be a simple light call to the server to collect the precise information it needs. Once we read the page, we can just close it and continue on the context of the main page already opened.

Let me know what you think about it. Thanks for the awesome improvement!

rcmoutinho avatar Apr 26 '24 13:04 rcmoutinho

thx for the feedback! It may take a while before I have this complete as I now have the following tasks:

  • update all table-row (navigation)clicks to allow open in new window
  • update all popup-menu (navigation)clicks to allow open in new window
  • provide the new tab/window with additional information (in the url) to indicate that it is a new tab/window
  • hide the on-page close-button on a new tab/window
  • hide the menu bar on a new tab/window (keep the "SaltGUI"-part visible?)
  • hide the jobs panel on a new tab/window
  • test with logout on main-window, what should happen on/with secondary windows?
  • update documentation
  • think about a visual clue for this and add that too

erwindon avatar Apr 26 '24 13:04 erwindon

Damn! Indeed, it's a lot, but you have a great plan.

Would hiding info prevent sending extra calls to the server or just not showing on the UI?

I am okay with or without the SaltGUI logo/menu. The information is the most essential part. I could execute the same module call to get it, but it would defeat the UI's purpose of making it easier/quicker to get the information.

rcmoutinho avatar Apr 26 '24 13:04 rcmoutinho

Thank you for making this possible.

This little feature makes SaltGUI become a power tool. When launching jobs, highstates, orch on dozens of minions at a time, I think this tool should limit the noise to a maximum. (events, find_job, etc.) And I think this here is the biggest part.

The list of tasks seem good. Good luck with the design as well!

gseguinbourgeois avatar Apr 26 '24 13:04 gseguinbourgeois

Would hiding info prevent sending extra calls to the server or just not showing on the UI?

As you have already seen, there is a separation between the pages and the panels. When you look at the code for the pages, you'll see that it is just the code to create the desired panels and add them. That code will be updated to only create+add the left-most panel and not create+add the jobs panel. Api-calls will only take place for the created+added panels.

erwindon avatar Apr 26 '24 14:04 erwindon

Perfect! Thanks for the extra details about the process.

rcmoutinho avatar Apr 26 '24 14:04 rcmoutinho

progress tracker:

  • [x] update all table-row (navigation)clicks to allow open in new window
  • [x] update all popup-menu (navigation)clicks to allow open in new window
  • [x] provide the new tab/window with additional information (in the url) to indicate that it is a new tab/window
  • [x] hide the on-page close-button on a new tab/window
  • [x] hide the menu bar on a new tab/window (keep the "SaltGUI"-part visible?)
  • [x] hide the jobs panel on a new tab/window
  • [x] test with logout on main-window, what should happen on/with secondary windows?
  • [x] update documentation
  • [x] prevent focus-border after CTRL-click on TR
  • [x] fix tooltips at top of window that (with the header removal) fall off the top of the window
  • [x] think about a visual clue for this and add that too (solved by creating a new issue: #575)

note: often git push --force is used on this branch, use git pull -r to update your workspace

erwindon avatar Apr 27 '24 00:04 erwindon

I tried CTRL-click, but that interfered too much with the "select-text" function of the browser.

Enabled it anyway. This has the fun-behavior that CTRL-click opens in a new tab without making that the current tab and ALT-click opens in a new tab which becomes the current tab. Note that that happens automatic, without specific instructions in SaltGUI. So it may behave different in different browsers. But at least Firefox+Chrome+Edge do this.

erwindon avatar Apr 27 '24 09:04 erwindon

Interesting! I remember that once I clicked it, it opened and activated the new tab. However, I think the default behavior on the browser for Alt is to "save link as". I will test all your changes first thing Monday morning and give you extra feedback if there is any.

Thanks again for the massive code changes to make it happen 🎉

rcmoutinho avatar Apr 27 '24 11:04 rcmoutinho

fix tooltips at top of window that (with the header removal) fall off the top of the window

solution is to show the header again, only the menu-items are now hidden in secondary tab/page.

erwindon avatar Apr 27 '24 18:04 erwindon

This is beautiful! Opening five job results simultaneously using CTRL is even better than ALT (considering it changes the page focus). Both approaches are working on my side 🎉

I enabled all notifications to ensure I update them as soon as possible once this is merged with the master. Thanks a lot again! 🤩

rcmoutinho avatar Apr 29 '24 13:04 rcmoutinho

Quality Gate Failed Quality Gate failed

Failed conditions
12.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

sonarqubecloud[bot] avatar Apr 29 '24 21:04 sonarqubecloud[bot]

@rcmoutinho I've handled the last two tasks. The visualization of this all is delegated to a separate issue #575. I found how to undo the table-cell-selection that happens on a ctrl-click in a table. This concludes the coding part of this issue/PR.

I plan to make a new release of SaltGUI after this issue and issue #571 are done.

Can you give this PR one final round of testing?

erwindon avatar Apr 29 '24 21:04 erwindon

@erwindon re-deployed the branch, and it continues to work great! Thanks a lot for all the work!

rcmoutinho avatar Apr 30 '24 13:04 rcmoutinho