-
Notifications
You must be signed in to change notification settings - Fork 56
Tooltips from component classes #327
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
base: master
Are you sure you want to change the base?
Tooltips from component classes #327
Conversation
DougReeder
left a comment
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.
LGTM, but does class Networked(HubsComponent) or class VideoTextureTarget(HubsComponent) need a tooltip?
|
Some of the tooltips are written in terms of adding something to the scene and some aren't. I am fine with merging this as-is, but it might be nice to write all the tooltips one way or the other. |
Exairnous
left a comment
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.
@Spiderguy-F
Thank you! This will really help a lot. I've added comments/suggestions/corrections.
These suggestions/corrections hopefully bring the tooltips in line with https://developer.blender.org/docs/features/interface/human_interface_guidelines/tooltips/ correct some inaccuracies, and make sure everything is consistent.
Also, the tooltip for the Video Texture Target component is missing.
Note: some of my suggestions could probably be further improved. If you or anyone has better wording, please suggest it.
@DougReeder
The Networked component won't need a tooltip because it can't be manually added, but the Video Texture Target should have one.
Some of the tooltips are written in terms of adding something to the scene and some aren't. I am fine with merging this as-is, but it might be nice to write all the tooltips one way or the other.
I agree that the tooltips should be kept as consistent as possible and I'm thinking that it mostly shouldn't reference adding again since we are already in the process of adding a Hubs component.
@Heather-Elizabeth-Dodds
Can you also take a look at this pull request, please?
Finally:
Since there are a couple files missing from the pull request at this time, I'll leave my suggestions for their tooltips here:
Audio:
'tooltip': 'Display an audio player that plays a file from a URL'
Video Texture Target:
'tooltip': 'Display video from the corresponding video texture source. A virtual screen'
| 'deps': ['networked'], | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Create an interactable hyperlink' |
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.
| 'tooltip': 'Create an interactable hyperlink' | |
| 'tooltip': 'Display a preview of a website or Hubs asset that can be interacted with (behavior depends on the type of link). This can be combined with other components to override the auto-generated preview' |
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.
'Display a preview of interactive website or Hubs asset. If combined with other components, overrides auto-generated preview.'
| 'icon': 'LIGHT_SUN', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'A light that emits parallel light in a specific direction, like the sun', |
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.
| 'tooltip': 'A light that emits parallel light in a specific direction, like the sun', | |
| 'tooltip': 'Emit parallel light in a specific direction, like the sun', |
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'm really torn on this clause, "like the sun", because I think of that as object in space emitting light in all directions. I realize THIS is comparing to what the sun feels like on Earth though. What about adding a bit more?
'Emit parallel light in a specific direction, like how the sun's rays appear on Earth'
| 'icon': 'MOD_SMOOTH', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Control a shapekey for an avatar based on mic volume' |
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.
| 'tooltip': 'Control a shapekey for an avatar based on mic volume' | |
| 'tooltip': 'Use the user's transmitted audio to animate a shape key on the avatar' |
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.
'User transmitted audio animates an avatar shape key'
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.
Since an avatar can have many shape keys and this only modifies one, I'd suggest:
"Use the user's transmitted audio to animate a shape key of the avatar"
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.
Blender notes said to avoid the word "Use" (and enable, active, and is) because they could be confused with Boolean operators.
I say, "pah"! Let's go with use. I think it fits in this case. :)
| 'icon': 'MOD_MIRROR', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Add a mirror plane to the room' |
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.
| 'tooltip': 'Add a mirror plane to the room' | |
| 'tooltip': 'Display a virtual, planar, mirror that will give a live reflection of avatars, in-room objects, and the environment' |
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.
Probably don't need that second comma.
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.
Clarification: I meant the comma after "planar". Sorry! Can we remove that one and add back in the Oxford comma after objects? @Spiderguy-F
| 'deps': ['networked'], | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Load a 3D model (glb) from a URL' |
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.
| 'tooltip': 'Load a 3D model (glb) from a URL' | |
| 'tooltip': 'Display a 3D model (glb) from a URL' |
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.
Nice. Short. Sweet. To the point.
| 'version': (1, 0, 1), | ||
| 'deps': ['visible'] | ||
| 'deps': ['visible'], | ||
| 'tooltip': 'Define the area where avatars can walk' |
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.
| 'tooltip': 'Define the area where avatars can walk' | |
| 'tooltip': 'Designate this as the mesh that determines where avatars can walk' |
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.
A little wordy but I don't think it can be trimmed without losing some meaning.
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 think "Area" is more user friendly, the concept of marking an area by creating a mesh is a bit abstract, if you're not familiar with the concept.
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.
@Spiderguy-F Potentially, but the component has to be added to a mesh, so if they aren't familiar with the concept already they're going to need to learn it 🙂
| 'icon': 'LIGHT_POINT', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'A punctual light source that emits light in all directions', |
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.
| 'tooltip': 'A punctual light source that emits light in all directions', | |
| 'tooltip': 'Emit light in all directions from a single point' |
|
I'm not done reviewing but I keep making comments about using plural forms of verbs. I'll go check that before I finish. [EDIT] I find two examples on the Blender Developer Tooltips using singular verb forms: Do: "Show in Viewport" and Therefore, I'll go back and edit my comments to the singular tense. [EDIT 2] Done. Bonus points: I didn't really know what some of these buttons did, so it was great to learn. |
…m/hubs-blender-exporter-tooltips into TooltipsFromClasses
Co-authored-by: Exairnous <[email protected]>
Co-authored-by: Exairnous <[email protected]>
Co-authored-by: Exairnous <[email protected]>
…m/hubs-blender-exporter-tooltips into TooltipsFromClasses
Exairnous
left a comment
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.
@Heather-Elizabeth-Dodds I left a few comments on your comments. Aside from the few things I pointed out, I'm okay with your suggestions for my suggestions. Although you do seem to have cut out a number of my articles (a, the, etc.) in what looks like an attempt to make the tooltips as short as possible?
| 'deps': ['networked'], | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Display a preview of interactive website or Hubs asset. If combined with other components, overrides auto-generated preview.' |
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.
@Heather-Elizabeth-Dodds This is slightly incorrect now. It's not the website or asset that's interactive (they may or may not be), but the link itself, e.g. if you click on the link it will open a website in a new tab, change your avatar, change the scene, etc. Also, the link doesn't override the auto-generated preview, it's the other components that will override the link's auto-generated preview.
| 'icon': 'MATERIAL', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'A reflection probe is a type of light probe that captures the environment around it and uses that information to create reflections on objects in the scene', |
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.
This isn't quite right. The first sentence is okay (though it feels a bit weird not to be mentioning reflections directly). But for the second sentence, it's actually the other way around; the environment is baked into an image to use in the reflection probe, and the probe just facilitates baking this image.
| 'icon': 'GRAPH', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Control the scale of an object or bone based on mic volume' |
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.
This component is almost identical to the Morph Audio Feedback component (which is why I phrased it similarly), except it scales the bone between two values instead of morphing a shape key between two values. I'm not against adding in an additional comment, but if we do, I'd add it to the Morph Audio Feedback tooltip as well.
| 'icon': 'MOD_PARTICLE_INSTANCE', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Spawn objects (glb) loaded from a URL by dragging' |
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 we add "by dragging on it" to the end? As it is, it doesn't inform the user how the object is duplicated, so to me, it sounds like I'd duplicate a copy by hovering over it, pressing spacebar, and cloning it (which doesn't work).
| 'icon': 'HIDE_OFF', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Hide this object from the camera' |
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.
It's arguable whether "force" is necessary, but since you can use it to show the bounds of things like audio zones which are normally invisible (while most other things in Hubs are defaulted to visible) I went with saying you're forcing the state. As to Blender's instructions, most of the time I'd agree with them, but in this case I feel this better describes things because the component contains the toggle and so this isn't describing the toggle directly (I could be wrong though).
| 'icon': 'IMAGE_PLANE', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Define mesh culling settings for this object', |
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.
True. Okay, but how about '...passes outside of the user's field of view'? This way it's a little clearer which field of view is being talked about.
| 'icon': 'MOD_MIRROR', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Display a virtual, planar, mirror that will give a live reflection of avatars, in-room objects and the environment' |
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.
@Heather-Elizabeth-Dodds Which second comma? The one after planar? It looks like @Spiderguy-F removed the one after objects (which is fine, but it was a perfectly valid Oxford comma 😛)
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.
In German there is no comma before and, unless it's followed by a complete sentence. No Oxford in Germany. But the comma after planar would also not be there, not even between virtual and planar, unless they were closely related, like big, strong guy as opposed to small German guy. As weird as that is.
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.
@Heather-Elizabeth-Dodds and I discussed the tooltips at the latest Hubs Documentation Session and have come up with some new wording which I've added to the relevant spots. I think we've now communicated all of our suggested wording changes for now and we'll see how things are after you've finished updating things. Thank you again for your work to add these, this will be a very helpful improvement.
| 'icon': 'MOD_PARTICLE_INSTANCE', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Spawn objects (glb) loaded from a URL by dragging' |
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.
Revised wording that @Heather-Elizabeth-Dodds and I came up with:
| 'tooltip': 'Spawn objects (glb) loaded from a URL by dragging' | |
| 'tooltip': 'Using an existing URL, generate an object (glb) that a user can optionally duplicate by dragging' |
| 'icon': 'MOD_SMOOTH', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': "Use the user's transmitted audio to animate a shape key of the avatar" |
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.
Revised wording that @Heather-Elizabeth-Dodds and I came up with:
| 'tooltip': "Use the user's transmitted audio to animate a shape key of the avatar" | |
| 'tooltip': "Use the user's transmitted audio to animate a shape key on the avatar. Larger numbers cause more movement | |
| " |
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.
isn't 'can optionally' a tautology?
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.
Almost! But that phrasing is often used.
| 'icon': 'GRAPH', | ||
| 'version': (1, 0, 0) | ||
| 'version': (1, 0, 0), | ||
| 'tooltip': 'Control the scale of an object or bone based on mic volume' |
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.
Revised wording that @Heather-Elizabeth-Dodds and I came up with:
| 'tooltip': 'Control the scale of an object or bone based on mic volume' | |
| 'tooltip': 'Use the user's transmitted audio to animate the scale of an object or bone in the avatar. Larger numbers cause more movement' |
GottfriedHofmann
left a comment
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.
Tested and all tooltips work fine. I did not find a technical inaccuracy in the tooltips.
|
@Spiderguy-F Thank you for the update. The documentation team has this on our radar again. It looks like there are still some unresolved comments/suggestions that haven't been integrated yet. Unless you disagree with them (in which case let us know), could you please add the changes from them to the PR? |
What?
Adds tooltips to the add hubs component operator
Why?
Feature request
Examples
Any hubs component on scene, objects or bones.
How to test
Hover with your mouse over an Add Component button and then over one of the components that can be added from the menu.
Open questions
I'm not a native speaker and am not sure if I described all components accurately. Feedback / corrections are appreciated.