feat(ContentNavigator): added more detailed path information #1597

kpatl1 · 2025-08-14T18:29:17Z

Summary

This change improves the user experience in the SAS Content Navigator by showing full hierarchical paths when hovering over files and folders, instead of just displaying the filename.

Key Changes:

Modified ContentDataProvider.getTreeItem() to fetch and display full path information in tooltips
Added fallback behavior - if full path retrieval fails, tooltips gracefully degrade to showing just the item name

Before:

Hovering over a file showed: /test1.sas

After:

Hovering over a file shows: /My Folder/Subfolder/test1.sas

Testing

-Verified tooltips show full paths for files in nested folder structures

Fixes #1577

scottdover

Hey @kpatl1! Thanks for taking a look at this. Only a couple things I noticed...

If you hover over SAS Server, it'll show a tooltip that isn't quite right. We could honestly probably update the implementation in RestServerAdapter.getPathOfItem and ItcServerAdapter.getPathOfItem to return an empty string for the root folder
Currently, the recycle bin has a path tooltip. Is this expected (is that a valid path)?
Last, the only concern I have is that this will be called once per file or folder. So if we have a folder w/ 50 files, that's 51 requests to load the files (looks like this is only an issue w/ sas content). Have we tested how this impacts things if we have lots of files (i.e. btw 50-100 files)? The good thing is vscode does a decent job at caching these items so we shouldn't call it over and over. This is probably a non-issue, but just wanted to call it out for resting

scottdover · 2025-08-14T19:37:56Z

Thinking about it further, you can just check if the path can be copied before you show the tooltip (i.e. checking item.contextValue.includes('copyPath'))

…only fetch valid paths

I, Kishan Patel <[email protected]>, hereby add my Signed-off-by to this commit: c0622a9 I, Kishan Patel <[email protected]>, hereby add my Signed-off-by to this commit: 64965fe Signed-off-by: Kishan Patel <[email protected]>

kpatl1 · 2025-08-14T21:42:10Z

Thinking about it further, you can just check if the path can be copied before you show the tooltip (i.e. checking item.contextValue.includes('copyPath'))

You're totally right. Thank you for pointing these things out to me. I have gone in and added the copyPath optimization to only show valid paths. Furthermore, I have added caching for the parent to allow for the path to be concatenated for every sibling after the original server call. I am not sure if I fully implemented it correctly, so I would love if you could take a look at it when you get a chance.

scnwwu · 2025-08-15T08:40:01Z

I think it's not only about the tooltip on the tree item. If you open a file from SAS Content/Server and hover on the editor tab header, you'll get the same tooltip.

It's more crucial here as user has no way to know where does this file come from. If there're 2 files with same name, there's no way to distinguish them.
VS Code displays the URI as tooltip by default. Ideally if we can make the uri field be a read-able path. The tooltip will be good everywhere.

scnwwu · 2025-08-15T08:51:46Z

I have the same concern with @scottdover about the network call. It's not only one call per file but n calls per file where n is the nested level, since the getPathOfItem call is recursive.
Like #1559, I think in most cases user expands the tree from root and the parents should be already known, instead of having to query back.

scottdover · 2025-08-15T13:08:56Z

I think it's not only about the tooltip on the tree item. If you open a file from SAS Content/Server and hover on the editor tab header, you'll get the same tooltip. It's more crucial here as user has no way to know where does this file come from. If there're 2 files with same name, there's no way to distinguish them. VS Code displays the URI as tooltip by default. Ideally if we can make the uri field be a read-able path. The tooltip will be good everywhere.

Ooh, I like this idea. I think this is probably the best path forward.

client/src/components/ContentNavigator/ContentDataProvider.ts

scottdover · 2025-08-15T13:17:41Z

client/src/components/ContentNavigator/ContentDataProvider.ts

+
+    if (!this.parentPathCache.has(parentUri)) {
+      try {
+        const fullPath = await this.model.getPathOfItem(item);


It may also be worth updating RestContentAdapter.getPathOfItem to handle its own caching since it's really the only ContentAdapter that requires it. And doing that w/n RestContentAdapter may make things easier w/r/t Wei's comment about using uri

…more efficent path retrieval

I, Kishan Patel <[email protected]>, hereby add my Signed-off-by to this commit: 05fb7f2 Signed-off-by: Kishan Patel <[email protected]>

kpatl1 · 2025-08-15T19:51:07Z

Made a new commit by changing the uri to contain the full path. I have also moved the caching into the RestContentAdapter. I cannot seem to get the content in SAS Server to show the full path, however, only when I view items in My Folder. Do you guys have any advice on how I could approach this?

scottdover · 2025-08-22T17:15:03Z

Made a new commit by changing the uri to contain the full path. I have also moved the caching into the RestContentAdapter. I cannot seem to get the content in SAS Server to show the full path, however, only when I view items in My Folder. Do you guys have any advice on how I could approach this?

Fwiw, sas server uris have a different structure and call getSasServerUri. But maybe I'm misunderstanding what you're saying

scottdover · 2025-08-22T17:16:56Z

client/src/connection/rest/util.ts

@@ -38,12 +38,18 @@ export const getResourceIdFromItem = (item: ContentItem): string | null => {
  return getLink(item.links, "GET", "self")?.uri || null;
 };

-export const getSasContentUri = (item: ContentItem, readOnly?: boolean): Uri =>
+export const getSasContentUri = (


Instead of having a nested ternary here, could we just always pass in fullPath. So the function would become something like

getSasContentUri(item: ContentItem, path: string, readOnly?: boolean)

and we'd just always use path

Would this not cause some of the times that this method is called to change into an async function because we would have to get the path of the item? I am just unsure why we cannot keep it as an optional parameter?

scottdover · 2025-08-22T17:24:41Z

client/src/connection/rest/RestContentAdapter.ts

+          if (enrichedItem.contextValue?.includes("copyPath")) {
+            await this.updateUriWithFullPath(enrichedItem);
+          }


It looks like the only time we don't call updateUriWithFullPath is when...

getting root items

moving items

I'd advocate for moving this call inside enrichWithDataProviderProperties (I know that make things a tiny bit more complicated since this function doesn't return a promise)

I agree. I will go ahead and refactor to allow for this.

scottdover · 2025-08-22T17:38:38Z

client/src/connection/rest/RestContentAdapter.ts

@@ -186,21 +202,59 @@ class RestContentAdapter implements ContentAdapter {
    if (!item) {
      return "";
    }
+    const baseKey = item.uri || item.id || item.name;


This seems like a lot. I'd probably update things such that we're setting the values here, and updating in calling functions where we know the full path might change (rename, move, delete, restore). So the general outline might be something like...

// In a function that changes the value this.pathCache.set(item.id, await this.calculatePathOfItem(item));

then the implementation would use the id for the cache key since that's mostly unchanging...

if (this.pathCache.has(item.id)) { return this.pathCache.get(item.id)!; } try { const path = await this.calculatePathOfItem(item, folderPathOnly); this.pathCache.set(item.id, path); return path; } catch { return ""; }

Would something like this work?

kpatl1

Made a new commit by changing the uri to contain the full path. I have also moved the caching into the RestContentAdapter. I cannot seem to get the content in SAS Server to show the full path, however, only when I view items in My Folder. Do you guys have any advice on how I could approach this?

Fwiw, sas server uris have a different structure and call getSasServerUri. But maybe I'm misunderstanding what you're saying

I understand. I think my confusion came from poor wording. When you hover over a file under the SAS Content folder, it does not resolve a path only the item.name. However, for files in My Folder it works, so would I need to utilize getSasServerUri for those files?

Signed-off-by: Kishan Patel <[email protected]>

feat(ContentNavigator): added more detailed path information

c0622a9

kpatl1 requested a review from scnwwu August 14, 2025 18:30

kpatl1 marked this pull request as ready for review August 14, 2025 18:30

scottdover reviewed Aug 14, 2025

View reviewed changes

Kishan Patel added 2 commits August 14, 2025 17:38

feat(ContentNavigator): added optimization with parent caching, also …

64965fe

…only fetch valid paths

kpatl1 assigned kpatl1 and unassigned kpatl1 Aug 14, 2025

scottdover reviewed Aug 15, 2025

View reviewed changes

client/src/components/ContentNavigator/ContentDataProvider.ts Outdated Show resolved Hide resolved

scottdover reviewed Aug 15, 2025

View reviewed changes

Kishan Patel added 4 commits August 15, 2025 14:14

feat(ContentNavigator): changed uri to have a full path to allow for …

05fb7f2

…more efficent path retrieval

DCO Remediation Commit for Kishan Patel <[email protected]>

fced7a6

I, Kishan Patel <[email protected]>, hereby add my Signed-off-by to this commit: 05fb7f2 Signed-off-by: Kishan Patel <[email protected]>

wip(ContentNavigator): Partial working modification

1e5a9b6

wip(ContentNavigator): Partial working modification

6e8b68c

scottdover reviewed Aug 22, 2025

View reviewed changes

kpatl1 commented Aug 22, 2025

View reviewed changes

kpatl1 added 2 commits August 22, 2025 17:27

wip(ContentNavigator) refactored enrichment logic

7a2bd53

Signed-off-by: Kishan Patel <[email protected]>

feat(ContentNavigator): improved caching

1545dc9

Signed-off-by: Kishan Patel <[email protected]>

feat(ContentNavigator): added more detailed path information #1597

Are you sure you want to change the base?

feat(ContentNavigator): added more detailed path information #1597

Uh oh!

Conversation

kpatl1 commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes:

Before:

After:

Testing

Uh oh!

scottdover left a comment

Choose a reason for hiding this comment

Uh oh!

scottdover commented Aug 14, 2025

Uh oh!

kpatl1 commented Aug 14, 2025

Uh oh!

scnwwu commented Aug 15, 2025

Uh oh!

scnwwu commented Aug 15, 2025

Uh oh!

scottdover commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

scottdover Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

kpatl1 commented Aug 15, 2025

Uh oh!

scottdover commented Aug 22, 2025

Uh oh!

scottdover Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

kpatl1 Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

scottdover Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

kpatl1 Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

scottdover Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

kpatl1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kpatl1 commented Aug 14, 2025 •

edited

Loading

scottdover commented Aug 15, 2025 •

edited

Loading