Skip to content

Adding SimpleButton + demo scene - #635

Merged
ms-RistoRK merged 27 commits into
MixedRealityToolkit:mainfrom
ms-RistoRK:MeshButtonIntegration
Feb 14, 2024
Merged

Adding SimpleButton + demo scene#635
ms-RistoRK merged 27 commits into
MixedRealityToolkit:mainfrom
ms-RistoRK:MeshButtonIntegration

Conversation

@ms-RistoRK

Copy link
Copy Markdown
Contributor

The SimpleButton structure is inspired in Mesh's MasterButtonRoot which is a simplified version of MRTK3's Action Button. Note: Materials (including shaders) are those existing in MRTK3 or its associated packages.

@ms-RistoRK
ms-RistoRK marked this pull request as ready for review February 8, 2024 00:39

@AMollis AMollis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please share some screen shots of the new scenes?

Comment thread org.mixedrealitytoolkit.uxcomponents/Editor/CreateElementMenus.cs Outdated
Comment thread org.mixedrealitytoolkit.uxcomponents/Editor/CreateElementMenus.cs Outdated
@AMollis
AMollis requested a review from a team February 8, 2024 01:01
@ms-RistoRK

Copy link
Copy Markdown
Contributor Author

Can you please share some screen shots of the new scenes?

Sure! Here is one taken from CanvasExampleSimpleButton scene in Editor:
image

@AMollis AMollis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you added a new feature please make sure the minor versions of 'uxcomponents' and 'standardassets' have been updated since the last release (https://github.com/orgs/MixedRealityToolkit/discussions/640).

It looks like 'standardassets' version needs to be updated from 3.0.2 to 3.1.0; edit 'standassets' package.json and CHANGELOG.md.

image

It looks like `uxcomponents' version is good (min version already update), but you need to update the dependency version in 'uxcompoents' to the latest 'standardassets' version:

image

Also the fonts in the screenshot looks off. Can you make sure it's using the same font and size as the "Action Button"

Comment thread org.mixedrealitytoolkit.uxcomponents/Editor/CreateElementMenus.cs Outdated
@AMollis AMollis added Type: Feature Request A request for a new feature that can be included with the next minor version release. Package: Standard Assets The Project standard assets package is impacted by this issue. labels Feb 9, 2024
@AMollis AMollis added the Package: UX Components The Project ux components package is impacted by this issue. label Feb 9, 2024
@ms-RistoRK

Copy link
Copy Markdown
Contributor Author

Also the fonts in the screenshot looks off. Can you make sure it's using the same font and size as the "Action Button"

Re: Fonts size, actually, that is a feature I was trying to add. The feature is that the Font Size would auto-adjust based on the containers height and always be 25% of the container height so that the initial label ("Simple\nButton") would always have 25% of the container's height as padding when a SimpleButton is created via Editor floating menu, as in:

        [MenuItem("GameObject/UI/MRTK/Simple Button (Experimental)")]
        private static void CreateSimpleButton(MenuCommand menuCommand)
        {
            // ...
            textMeshProUGUI.fontSize = simpleButtonContentGameObject.GetComponent<RectTransform>().rect.height / 4;
            // ...
        }

... although I notice no other UIElement does font size auto-adjustment so I removed it in the last commit and made all SimpleButtons' Font size the same as ActionButtons'.
The scene now looks like this:
image
I'll work in updating the package references next.

@ms-RistoRK

Copy link
Copy Markdown
Contributor Author

It looks like 'standardassets' version needs to be updated from 3.0.2 to 3.1.0; edit 'standassets' package.json and CHANGELOG.md.

Done in latest commit.

Comment thread org.mixedrealitytoolkit.standardassets/CHANGELOG.md
@AMollis

AMollis commented Feb 10, 2024

Copy link
Copy Markdown
Contributor

Also the fonts in the screenshot looks off. Can you make sure it's using the same font and size as the "Action Button"

Re: Fonts size, actually, that is a feature I was trying to add. The feature is that the Font Size would auto-adjust based on the containers height and always be 25% of the container height so that the initial label ("Simple\nButton") would always have 25% of the container's height as padding when a SimpleButton is created via Editor floating menu, as in:

        [MenuItem("GameObject/UI/MRTK/Simple Button (Experimental)")]
        private static void CreateSimpleButton(MenuCommand menuCommand)
        {
            // ...
            textMeshProUGUI.fontSize = simpleButtonContentGameObject.GetComponent<RectTransform>().rect.height / 4;
            // ...
        }

... although I notice no other UIElement does font size auto-adjustment so I removed it in the last commit and made all SimpleButtons' Font size the same as ActionButtons'. The scene now looks like this: image I'll work in updating the package references next.

Yikes that font size does not look great. I suggest we change the default label to "Label", and use the same font settings has the Action Button...does this help?

image
@ms-RistoRK
ms-RistoRK requested a review from AMollis February 13, 2024 23:46
@ms-RistoRK

Copy link
Copy Markdown
Contributor Author

The updated demo scene (CanvasExampleSimpleActionButton) now looks like this:
image

@AMollis AMollis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an "MRTK Issue" to add "StateVisualizer" to the new "Simplified Button"

@ms-RistoRK
ms-RistoRK merged commit 60a315d into MixedRealityToolkit:main Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Package: Standard Assets The Project standard assets package is impacted by this issue. Package: UX Components The Project ux components package is impacted by this issue. Type: Feature Request A request for a new feature that can be included with the next minor version release.

2 participants