Project

General

Profile

Actions

Bug #21200

closed

Breadcrumbs issues

Added by Lisa Knox 5 months ago. Updated 3 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Story points:
-
Release relationship:
Auto

Description

  • When navigating to a user from groups page, breadcrumbs does not show username. This happens whether or not the page is found/you have permission to view. The result is that you could be looking at another user's page without any indication that this was the case outside of the uuid in the url bar.
  • When viewing a user's page, if I click the parent breadcrumb (which is actually the last breadcrumb because of the above bug) it navigates to "shared with me" instead of the group. Might be related to https://dev.arvados.org/issues/20891
  • The side panel has a new item with the name "Shell Access," but when navigating there there the breadcrumb has no icon and says "Virtual Machines." it is the only place in Workbench2 (to my knowledge) where you can click a nav button that says one thing and you arrive at a page that says a different thing. Suggest using the same name and icon in both places.

Subtasks 1 (0 open1 closed)

Arvados - Task #21244: ReviewResolvedStephen Smith12/20/2023Actions
Actions #1

Updated by Peter Amstutz 5 months ago

  • Target version set to Development 2023-11-29 sprint
  • Assigned To set to Lisa Knox
Actions #2

Updated by Lisa Knox 5 months ago

  • Change all instances of "Virtual Machines" text in UI to read "Shell Access"
  • Add terminal Icon to Breadcrumbs for Shell Access
Actions #3

Updated by Peter Amstutz 5 months ago

  • Target version changed from Development 2023-11-29 sprint to Development 2024-01-03 sprint
Actions #4

Updated by Lisa Knox 4 months ago

Changes at arvados|2efa65c6c45d20d3f0a1ad1a82511ba82f917cd3 branch 21200-breadcrumbs-issues
developer-tests-workbench2: #1418

  • All agreed upon points are implemented / addressed.
    • done
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • N/A
  • Code is tested and passing, both automated and manual, what manual testing was done is described
    • tested as regular and admin user
  • Documentation has been updated.
    • N/A
  • Behaves appropriately at the intended scale (describe intended scale).
    • intended scale is one user
  • Considered backwards and forwards compatibility issues between client and server.
    • N/A
  • Follows our coding standards and GUI style guidelines.
    • done
Actions #5

Updated by Stephen Smith 4 months ago

This one is a bit tricky since I fixed some related stuff in my instance types panel, mainly refactoring the setBreadcrumbs for a few panels into breadcrumbs-actions and adding the same Shell Access navigation handlers to navigation-actions.ts (so when you merge there will be duplicates). This is also complicated since I started working in the main arvados repo under services/workbench2 instead of the arvados-workbench2 repo. I think the easiest thing to do is to use Tom's instructions to copy your branch into a branch on the main arvados repo and then merge main into your branch there to resolve these couple of conflicts: https://dev.arvados.org/projects/arvados/wiki/Moving_branches_from_arvados-workbench2_to_arvados_repo

  • For the user breadcrumb, I'm sometime seeing that the user breadcrumb is added when clicking on a user from the groups panel, but sometimes it navigates to the user's project and the user breadcrumb is missing - I'm testing this on pirca and just clicking back and forth on the top 3 users in the all users group
  • Activating side panel menu nodes for various pages is usually done in workbench-actions by calling activateSidePanelTreeItem, so in account-menu.tsx and admin-menu.tsx it would be good to remove the calls to ACTIVATE_TREE_PICKER_NODE and instead add a dispatch<any>(activateSidePanelTreeItem(SidePanelTreeCategory.SHELL_ACCESS)); to the loadVirtualMachines function, similar to the loadSharedWithMe function - it may not make sense to add the same to the loadVirtualMachinesAdmin function though since there isn't a Shell Access Admin side panel item and that one item only goes to the user shell access page, it's currently a bit confusing when you go to the Shell Access Admin page and the Shell Access left panel item is highlighted, but when you click on it it takes you to the user Shell Access page
  • In my branch I also refactored the setBreadcrumbs in workbench-actions into their own functions following the example of other load* functions, so when you merge you can just keep those calls and rename the VIRTUAL_MACHINES_USER_PANEL_LABEL and VIRTUAL_MACHINES_ADMIN_PANEL_LABEL in breadcrumbs-actions.ts and change their values to be correct, as well as change the icons there. I noticed you didn't pass in a UUID to the setBreadcrumbs call, that's related to being able to click on the breadcrumb and have it trigger the correct navigateTo in navigation-action.ts, the switch case there needs to line up with the UUID on the breadcrumb so if you press F2 to refactor the const names it should be fine because I did that in my usage
  • It seems like there's a special case in the side-panel-collapsed for the shell access label - since it should be named Shell Access everywhere, I think we should be able to get rid of that?
Actions #6

Updated by Lisa Knox 4 months ago

> when you merge there will be duplicates

  • duplicates de-duplicated

copy your branch into a branch on the main arvados repo...

  • copied

For the user breadcrumb, I'm sometime seeing that the user breadcrumb is added...

  • user breadcrumb is always added now, and when clicking a user, navigates to user profile page instead of that user's project page as per discussion

Activating side panel menu nodes...

  • done
  • In my branch I also refactored...
  • done

It seems like there's a special case...

  • removed
Actions #7

Updated by Stephen Smith 4 months ago

Just a few more things:

  • In navigation-action.ts:L84-86 there is a duplicate shell access case there, I think you can just remove that one and remove the const import for VIRTUAL_MACHINES_USER_PANEL_LABEL
  • I probably should have noticed before but since there's now a SHELL_ACCESS enum on SidePanelTreeCategory, it makes more sense to remove the VIRTUAL_MACHINES_USER_PANEL_LABEL definition in breadcrumb-actions.ts:L309 and replace all uses with SidePanelTreeCategory.SHELL_ACCESS, to be more in line with other pages that are in the side panel (and because this way the switch case in the first bullet point is checking for the same value that's being used in the breadcrumb uuid)
  • In workbench-actions.ts:L750 I think it would be better to call activateSidePanelTreeItem instead of ACTIVATE_TREE_PICKER_NODE directly, that way any future changes to activateSidePanelTreeItem or refactors will be easier since all the other methods there use activateSidePanelTreeItem
  • Also I noticed that the admin menu item for the "Shell access admin" page is labeled "Shell Access" instead of "Shell Access Admin"
Actions #8

Updated by Peter Amstutz 4 months ago

  • Target version changed from Development 2024-01-03 sprint to Development 2024-01-17 sprint
Actions #9

Updated by Lisa Knox 4 months ago

In navigation-action.ts:L84-86 there is a duplicate shell access case there...

  • removed

I probably should have noticed before but...

  • changed so that all SHELL_ACCESS (semantic) references point to the same (memory) reference, not just the same value

In workbench-actions.ts:L750 I think it would be better to call activateSidePanelTreeItem ...

  • done, and good catch

Also I noticed that the admin menu item for the "Shell access admin"...

  • left as-is per discussion
Actions #10

Updated by Stephen Smith 4 months ago

This lgtm!

Actions #11

Updated by Anonymous 4 months ago

  • Status changed from New to Resolved
Actions #12

Updated by Peter Amstutz 3 months ago

  • Release set to 69
Actions

Also available in: Atom PDF