-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include type in tooltip #287
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let someone else review the implementation/code itself, but I like this from a UX perspective. :) It keeps the slightly technical information in the tooltip where we're already exposing that sort of thing, and displays it consistently. 👍🏻
In the spirit of progressive disclosure, it can help learners (or mentors/instructors!) better understand how/why things work the way they do, without overloading the block UI itself.
if definition.variant_type == Variant.Type.TYPE_NIL: | ||
return definition.description | ||
elif definition.variant_type == Variant.Type.TYPE_BOOL: | ||
variant_as_string = "Boolean" | ||
elif definition.variant_type == Variant.Type.TYPE_INT: | ||
variant_as_string = "Integer" | ||
elif definition.variant_type == Variant.Type.TYPE_FLOAT: | ||
variant_as_string = "Float" | ||
elif definition.variant_type == Variant.Type.TYPE_STRING: | ||
variant_as_string = "String" | ||
elif definition.variant_type == Variant.Type.TYPE_VECTOR2: | ||
variant_as_string = "Vector2" | ||
elif definition.variant_type == Variant.Type.TYPE_COLOR: | ||
variant_as_string = "Color" | ||
elif definition.variant_type == Variant.Type.TYPE_NODE_PATH: | ||
variant_as_string = "NodePath" | ||
elif definition.variant_type == Variant.Type.TYPE_OBJECT: | ||
variant_as_string = "Object" | ||
elif definition.variant_type == Variant.Type.TYPE_CALLABLE: | ||
variant_as_string = "Callable" | ||
elif definition.variant_type == Variant.Type.TYPE_SIGNAL: | ||
variant_as_string = "Signal" | ||
elif definition.variant_type == Variant.Type.TYPE_DICTIONARY: | ||
variant_as_string = "Dictionary" | ||
elif definition.variant_type == Variant.Type.TYPE_ARRAY: | ||
variant_as_string = "Array" | ||
else: | ||
variant_as_string = "Undefined" | ||
|
||
return definition.description + "\n\nType: [b]" + variant_as_string + "[/b]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you instead do this:
if definition.variant_type == Variant.Type.TYPE_NIL: | |
return definition.description | |
elif definition.variant_type == Variant.Type.TYPE_BOOL: | |
variant_as_string = "Boolean" | |
elif definition.variant_type == Variant.Type.TYPE_INT: | |
variant_as_string = "Integer" | |
elif definition.variant_type == Variant.Type.TYPE_FLOAT: | |
variant_as_string = "Float" | |
elif definition.variant_type == Variant.Type.TYPE_STRING: | |
variant_as_string = "String" | |
elif definition.variant_type == Variant.Type.TYPE_VECTOR2: | |
variant_as_string = "Vector2" | |
elif definition.variant_type == Variant.Type.TYPE_COLOR: | |
variant_as_string = "Color" | |
elif definition.variant_type == Variant.Type.TYPE_NODE_PATH: | |
variant_as_string = "NodePath" | |
elif definition.variant_type == Variant.Type.TYPE_OBJECT: | |
variant_as_string = "Object" | |
elif definition.variant_type == Variant.Type.TYPE_CALLABLE: | |
variant_as_string = "Callable" | |
elif definition.variant_type == Variant.Type.TYPE_SIGNAL: | |
variant_as_string = "Signal" | |
elif definition.variant_type == Variant.Type.TYPE_DICTIONARY: | |
variant_as_string = "Dictionary" | |
elif definition.variant_type == Variant.Type.TYPE_ARRAY: | |
variant_as_string = "Array" | |
else: | |
variant_as_string = "Undefined" | |
return definition.description + "\n\nType: [b]" + variant_as_string + "[/b]" | |
if definition.variant_type == Variant.Type.TYPE_NIL: | |
return definition.description | |
return definition.description + "\n\nType: [b]" + type_string(definition.variant_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except perhaps not appending the type in the NIL case, as you already did. I'll edit my suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for amending this. When I got back to my computer to test the branch I thought it would be clearer to use a format string, so I took the liberty of making that change in your patch.
This is a really nice improvement. Thanks for thinking of it & submitting it! |
The variant name as a string is appended to the description. If null, then no type is shown
Helping with #271 (comment)