UI: Tooltips for Assets #106189

Harley Acheson · 2023-03-27T19:07:58+02:00

Harley Acheson commented

2023-03-27 19:07:58 +02:00

Show tooltips with minimal data for Asset Browser items.

We don't currently show any tooltips for Asset Browser items. This can be problematic because as the thumbnail size decreases you are more likely to have the name truncated.

This PR adds these tooltips with minimum code changes and with minimal information. This is meant to be an easy and uncontentious solution for 3.6 (mostly because of Assets) that I could (propose to) extend further for version 4.0.

Following shows a tooltip for an Asset, showing just name and description (if available). Note the utility of seeing the full name in this situation. We could also show license, copyright, and author but these items seemed less immediately necessary and can be found in the side panel if needed.

Show tooltips with minimal data for Asset Browser items. --- We don't currently show any tooltips for Asset Browser items. This can be problematic because as the thumbnail size decreases you are more likely to have the name truncated. This PR adds these tooltips with minimum code changes and with minimal information. This is meant to be an easy and uncontentious solution for **3.6** (mostly because of Assets) that I could (propose to) extend further for version 4.0. Following shows a tooltip for an Asset, showing just name and description (if available). Note the utility of seeing the full name in this situation. We could also show license, copyright, and author but these items seemed less immediately necessary and can be found in the side panel if needed. ![image](/attachments/c8e42cea-cf22-4353-8a71-c7bdf3178aa2)

image.png

107 KiB

image.png

78 KiB

image.png

50 KiB

❤️ 7 🎉 2

Harley Acheson added this to the User Interface project 2023-03-27 19:09:46 +02:00

Harley Acheson requested review from Julian Eisel 2023-03-27 19:09:57 +02:00

Harley Acheson force-pushed TooltipsSimple from f1e4dee2d0 to b5fbe189c5

2023-03-28 18:57:58 +02:00

Compare

Harley Acheson referenced this pull request

2023-03-28 23:02:46 +02:00

Asset Browser/File Browser: Add display name tooltip to tile view #104693

Isaac Gicheha commented

2023-04-05 09:15:13 +02:00

First-time contributor

For the file system tooltip, should it only have the blend file name rather than having the file path? In a scenario where the path is long and also the path is already at the top.

For the file system tooltip, should it only have the blend file name rather than having the file path? In a scenario where the path is long and also the path is already at the top. > ![](https://projects.blender.org/attachments/a0810972-00cc-4748-a799-77ae802043d6)

Harley Acheson force-pushed TooltipsSimple from b5fbe189c5 to 56d33ddb9f

2023-04-06 01:54:45 +02:00

Compare

Harley Acheson commented

2023-04-06 01:58:11 +02:00

@shwaky - For the file system tooltip, should it only have the blend file name rather than having the file path? In a scenario where the path is long and also the path is already at the top.

Good point. There is a situation where you would still want the file path, and that is when you have recursion turned on.

So have update the PR to only show the path if recursion_level > 0. I have updated the capture in the first comment to show what it looks like in most situations.

> @shwaky - For the file system tooltip, should it only have the blend file name rather than having the file path? In a scenario where the path is long and also the path is already at the top. Good point. There is a situation where you would still want the file path, and that is when you have recursion turned on. So have update the PR to only show the path if recursion_level > 0. I have updated the capture in the first comment to show what it looks like in most situations.

Julian Eisel requested changes 2023-04-11 16:25:00 +02:00

Julian Eisel left a comment

Let's treat asset and file tooltips as separate patches. For assets things are straight forward, for files further discussion may be needed (like which information to display there, formatting, etc). I'm also not convinced the tooltip is the best place for this information, we could also display file information in the sidebar or in some extra region (like some other file browsers do).

source/blender/editors/space_file/file_draw.cc Outdated

						
				@ -112,0 +139,4 @@

				  char description[2048];

				  size_t slen = 0;

				  if (file->asset) {

Julian Eisel commented

2023-04-11 11:54:50 +02:00

Let's not have functions with such big-ish if-else if-else blocks. Simply set a dedicated tooltip callback for assets, this can be much more concise then. All we need here is the asset representation I think, all the other variables/data just add noise.

Let's not have functions with such big-ish `if`-`else if`-`else` blocks. Simply set a dedicated tooltip callback for assets, this can be much more concise then. All we need here is the asset representation I think, all the other variables/data just add noise.

Harley marked this conversation as resolved

source/blender/editors/space_file/file_draw.cc Outdated

						
				@ -112,0 +179,4 @@

				    if (file->typeflag & (FILE_TYPE_BLENDER | FILE_TYPE_BLENDER_BACKUP) &&

				        !(file->attributes & FILE_ATTR_OFFLINE)) {

				      /* Load Blender version directly from the file. */

				      short version = BLO_version_from_file(full_path);

Julian Eisel commented

2023-04-11 12:03:51 +02:00

While using "minimal" .blend reading, BLO_version_from_file() still has to open a .blend file from disk, which can't just be assumed to be entirely trivial. E.g. on network drives, displaying this tooltip could freeze Blender for seconds, since everything happens on the main thread. We could query the version on file list loading (which uses a thread).

Opinion @mont29 / @ideasman42?

While using "minimal" .blend reading, `BLO_version_from_file()` still has to open a .blend file from disk, which can't just be assumed to be entirely trivial. E.g. on network drives, displaying this tooltip could freeze Blender for seconds, since everything happens on the main thread. We could query the version on file list loading (which uses a thread). Opinion @mont29 / @ideasman42?

Harley marked this conversation as resolved

Harley Acheson changed title from ~~UI: Tooltips for Assets and File Browser Thumbnails~~ to WIP UI: Tooltips for Assets

2023-04-11 17:40:03 +02:00

Harley Acheson force-pushed TooltipsSimple from 56d33ddb9f to 149b6dc338

2023-04-11 18:49:08 +02:00

Compare

Harley Acheson changed title from ~~WIP UI: Tooltips for Assets~~ to UI: Tooltips for Assets

2023-04-11 18:50:09 +02:00

Harley Acheson commented

2023-04-11 18:52:02 +02:00

@JulianEisel - Let's treat asset and file tooltips as separate patches

Done. This PR is now greatly simplified by only dealing with asset tooltips.

> @JulianEisel - Let's treat asset and file tooltips as separate patches Done. This PR is now greatly simplified by only dealing with asset tooltips.

👍 1

Julian Eisel reviewed 2023-04-11 18:58:19 +02:00

source/blender/editors/space_file/file_draw.cc Outdated

						
				@ -114,0 +117,4 @@

				static char *file_draw_asset_tooltip_func(bContext * /*C*/, void *argN, const char * /*tip*/)

				{

				  const AssetRepresentation *asset = static_cast<AssetRepresentation *>(argN);

				  char description[2048];

Julian Eisel commented

2023-04-11 18:58:19 +02:00

I would suggest using std::string instead of a fixed size buffer. Removes the length limit and makes the code a lot more readable. Check node_errors_tooltip_fn() as reference.
(Sorry for only bringing this up now)

I would suggest using `std::string` instead of a fixed size buffer. Removes the length limit and makes the code a lot more readable. Check `node_errors_tooltip_fn()` as reference. (Sorry for only bringing this up now)

Harley marked this conversation as resolved

Harley Acheson commented

2023-04-11 19:39:25 +02:00

@JulianEisel - ...using std::string...a lot more readable.

Yes, can't argue with that; looks much nicer. Thanks!

@JulianEisel - ...using std::string...a lot more readable. Yes, can't argue with that; looks much nicer. Thanks!

Julian Eisel approved these changes 2023-04-12 13:19:13 +02:00

source/blender/editors/space_file/file_draw.cc Outdated

						
				@ -114,0 +117,4 @@

				static char *file_draw_asset_tooltip_func(bContext * /*C*/, void *argN, const char * /*tip*/)

				{

				  const AssetRepresentation *asset = static_cast<AssetRepresentation *>(argN);

				  std::string complete_string (AS_asset_representation_name_get(asset));

Julian Eisel commented

2023-04-12 13:17:48 +02:00

Would initialize like this std::string complete_string = AS_asset_representation_name_get(asset);.

Would initialize like this `std::string complete_string = AS_asset_representation_name_get(asset);`.

Harley marked this conversation as resolved

Harley Acheson force-pushed TooltipsSimple from 613c0584c2 to da31cc9b1b

2023-04-12 18:42:46 +02:00

Compare

Harley Acheson merged commit 0ee0e8c0d4 into main

2023-04-12 18:44:59 +02:00

Harley Acheson referenced this issue from a commit

2023-04-12 18:44:59 +02:00

UI: Tooltips for Assets

Harley Acheson deleted branch TooltipsSimple

2023-04-12 18:45:00 +02:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

UI: Tooltips for Assets #106189