feat(AssetManager): Allow editing asset names without a popup

[rexy712]

This allows you to click on the name of an asset or folder and simply modify the name in place, rather than having a popup appear to change the name. You can complete editing by simply pressing Enter or clicking outside of the editable text.

The popup system works fine, but I find it to be cumbersome and less intuitive. It also has the issue of starting with a blank text field instead of the current name of the asset/folder. Most of the time when I'm editing a name, it's to fix a small typo I made, so having to start at a blank slate each time is frustrating.

While this may not be a perfect implementation, it's more similar to what most people would expect after using a modern file browser. I also do not remove the Rename functionality in the context menu in this change so that not everyone is forced into using this system if they're used to the old way.

2024-10-28.18-02-58.mp4

[Kruptein]

It's interesting because to me this is less intuitive :D the effect is very subtle which makes it easy to accidentally click on a name when you just want to select something.

I'm also very curious as to your statement

It's more similar to what most people would expect after using a modern file browser

I personally haven't seen this being used anywhere as a default :p Neither windows explorer or the file managers I'm used to on linux have this behaviour by default, nor do online file storage solutions like a google drive. I'm aware they can usually be configured to do so however, but I wouldn't assume this to be done by most people.

That said, I do agree that the context menu and action popups can use some UI/UX improvements.

I think the bigger issue however is that the extensions are still present on the asset names (#1022), which, I'm going to guess, is the biggest reason why you might need to rename them so often. If that were fixed, would you still have a need to rename assets that often causing the context menu to remain a bother?

[rexy712]

It's interesting because to me this is less intuitive :D the effect is very subtle which makes it easy to accidentally click on a name when you just want to select something.

Hm I hadn't thought it would be easy to accidentally click. I always aim for the image when trying to select, but fair point.

I personally haven't seen this being used anywhere as a default :p Neither windows explorer or the file managers I'm used to on linux have this behaviour by default, nor do online file storage solutions like a google drive.

I suppose I would say most file managers I've used allow a slow doubleclick to edit the name. I went for what seemed easiest to me, which is similar but not the same as a file manager.

I think the bigger issue however is that the extensions are still present on the asset names (#1022), which, I'm going to guess, is the biggest reason why you might need to rename them so often. If that were fixed, would you still have a need to rename assets that often causing the context menu to remain a bother?

That would take care of a good amount of the renaming certainly.

Perhaps a better implementation would be a combination of the old method and this. Instead of clicking the title to initiate a rename, use the context menu option to make the title editable in place. Would that be worth trying?

[Kruptein]

Perhaps a better implementation would be a combination of the old method and this. Instead of clicking the title to initiate a rename, use the context menu option to make the title editable in place. Would that be worth trying?

That would definitely be an upgrade over what it currently does yeah,
so I think that's a good compromise for now.

[rexy712]

I changed the way this works to now doing a right-click->rename to make the title editable in-place. The newly editable title gains focus and the text is all selected by default.

One issue I have is that there is a linter error I cannot seem to figure out how to fix. It has to do with a part of the file I didn't change. The remove function in client/src/assetManager/AssetContext.vue was completely untouched, but now its inclusion in the sections array causes a Promise-returning function provided to property where a void return was expected @typescript-eslint/no-misused-promises linting error. I checked the definition of the types in that array, and it should still be fine, as the functions are allowed to return a void or Promise<void>.

2024-10-30.14-07-39.mp4

[Kruptein]

So what is happening with the lint error:
There are 3 sections, they all have roughly the same shape, except 'share' which also has a disabled property. Previously it so happened that the 2 sections that did not specify disabled all were async and the one that did specify disabled was sync, so typescript autogenerated a distinct set of types where the presence of disabled dictates whether the action is sync or async.

Now this is no longer the case and apparently it's not happy. I'm not entirely sure why it's not just making the action type a union, but that's what's happening.

The solution is rather easy, explicitly tell the computed what type it is:

const sections = computed<Section[]>(() => [
   ...
]);

You can import Section from the core types (see also ContextMenu props)

[Kruptein]

Works well! I have a bunch of comments but they're almost all purely nitpicking about small stylistic thing.

I recommend to first check my comment on the canRename function as some of my other comments may no longer be relevant if you agree with my feedback there.

[rexy712]

I'm not entirely sure why it's not just making the action type a union, but that's what's happening.

The solution is rather easy, explicitly tell the computed what type it is:

const sections = computed<Section[]>(() => [
   ...
]);

Thanks for the solution! It was driving me crazy. This gives me flashbacks to C++ template errors and trying to debug those, but this time in a much less familiar language.

[Kruptein]

Last miniscule change :D