Skip to content
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

Padded ItemStatus Text #29560

Merged
merged 6 commits into from
Sep 19, 2024

Conversation

Willhelm53
Copy link
Contributor

About the PR

Adds a little margin to the ItemStatus text. Fixes #27756.

Why / Balance

Literally unplayable, WizDen plz fix
Minor graphical fix.

Technical details

"No held item" can have a little margin as a treat.

Media

(Showcase not needed. Before/After included for ease of review.)

Before:
image

After:
image

  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

Changelog

n/a

@github-actions github-actions bot added Changes: UI Can be reviewed or fixed by people who are knowledgeable with UI design No C# For things that don't need code. labels Jun 29, 2024
Copy link
Member

@ShadowCommander ShadowCommander left a comment

Choose a reason for hiding this comment

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

Add this to StyleNano for ItemStatusNotHeld.

@Willhelm53
Copy link
Contributor Author

Thanks! Let me know if I missed anything else.

image

@github-actions github-actions bot added the Status: Needs Review This PR requires new reviews before it can be merged. label Jun 29, 2024
Copy link
Member

@ShadowCommander ShadowCommander left a comment

Choose a reason for hiding this comment

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

Do the same for ItemStatus and check if it works if you remove the Margin from the xaml.

If it doesn't work, then just use the margin in the xaml.

Also nice commits.

@metalgearsloth metalgearsloth added Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. and removed Status: Needs Review This PR requires new reviews before it can be merged. labels Jun 29, 2024
@Willhelm53
Copy link
Contributor Author

When I tried StyleNano only, it worked for ItemStatusNotHeld but not for ItemStatus. Sticking with the XAML for now because it works for both. Thanks for pointing me towards StyleNano! This one's ready for review any time.

@ShadowCommander
Copy link
Member

ShadowCommander commented Jun 29, 2024

It does work in StyleNano, but you have to convert the ItemStatus one from the old way to the new way. ItemStatusNotHeld is using the new way.

@Plykiya Plykiya added Status: Needs Review This PR requires new reviews before it can be merged. and removed Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. labels Jul 23, 2024
@Plykiya
Copy link
Contributor

Plykiya commented Jul 23, 2024

I don't know how to suggest this as a change, but they want you to do this in StyleNano.cs

             Element()
                .Class(StyleClassItemStatusNotHeld)
                .Prop("font", notoSansItalic10)
                .Prop("font-color", ItemStatusNotHeldColor)
                .Prop(nameof(Control.Margin), new Thickness(4, 0, 0, 2)),

            Element()
                .Class(StyleClassItemStatus)
                .Prop(nameof(RichTextLabel.LineHeightScale), 0.7f)
                .Prop(nameof(Control.Margin), new Thickness(4, 0, 0, 2)),

image

image

@metalgearsloth metalgearsloth enabled auto-merge (squash) September 19, 2024 07:51
@metalgearsloth metalgearsloth merged commit 3acf6b9 into space-wizards:master Sep 19, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: UI Can be reviewed or fixed by people who are knowledgeable with UI design No C# For things that don't need code. Status: Needs Review This PR requires new reviews before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Itemstatus left side text needs slightly more padding
4 participants