Skip to content

Commit 2190b92

Browse files
fix(core,ui): address code review feedback
- Add null guard to BootstrapText.SafeFormat for null templates - Make JTabView._maxTabsPerRow readonly - Use evt.currentTarget instead of evt.target in JTabView handlers - Add SafeFormat null template test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
1 parent 4decf9c commit 2190b92

4 files changed

Lines changed: 21 additions & 9 deletions

File tree

UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/BootstrapText.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,15 @@ public struct BootstrapText
151151

152152
/// <summary>
153153
/// Safe wrapper around <see cref="string.Format(string, object[])"/> that falls back
154-
/// to the raw template if the user-edited format string is malformed.
154+
/// to a safe value if the user-edited format string is malformed or null.
155155
/// </summary>
156156
public static string SafeFormat(string template, params object[] args)
157157
{
158+
if (string.IsNullOrEmpty(template))
159+
{
160+
return string.Empty;
161+
}
162+
158163
try
159164
{
160165
return string.Format(template, args);

UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Components/Navigation/JTabView.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public class JTabView : VisualElement
4141
private readonly List<Label> _tabButtons = new();
4242
private readonly List<VisualElement> _contentPanels = new();
4343
private int _selectedIndex = -1;
44-
private int _maxTabsPerRow;
44+
private readonly int _maxTabsPerRow;
4545

4646
/// <summary>
4747
/// Creates a new tab view with a tab bar and content area.
@@ -202,7 +202,7 @@ private static void ApplyInactiveStyle(Label tab)
202202

203203
private static void OnTabClicked(MouseDownEvent evt)
204204
{
205-
if (evt.target is Label tab && tab.userData is int index)
205+
if (evt.currentTarget is Label tab && tab.userData is int index)
206206
{
207207
// Walk up to find the JTabView parent
208208
var parent = tab.parent?.parent;
@@ -215,15 +215,15 @@ private static void OnTabClicked(MouseDownEvent evt)
215215

216216
private void OnTabMouseEnter(MouseEnterEvent evt)
217217
{
218-
if (evt.target is Label tab && tab.userData is int index && index != _selectedIndex)
218+
if (evt.currentTarget is Label tab && tab.userData is int index && index != _selectedIndex)
219219
{
220220
tab.style.backgroundColor = Tokens.Colors.BgHover;
221221
}
222222
}
223223

224224
private void OnTabMouseLeave(MouseLeaveEvent evt)
225225
{
226-
if (evt.target is Label tab && tab.userData is int index && index != _selectedIndex)
226+
if (evt.currentTarget is Label tab && tab.userData is int index && index != _selectedIndex)
227227
{
228228
tab.style.backgroundColor = StyleKeyword.None;
229229
}

UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Components/Navigation/JTabViewTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ public void OnTabClicked_SelectsCorrectTab()
570570
Assert.IsNotNull(method, "OnTabClicked method should exist");
571571

572572
var evt = (MouseDownEvent)Activator.CreateInstance(typeof(MouseDownEvent), true);
573-
evt.target = secondTab;
573+
evt.currentTarget = secondTab;
574574
method.Invoke(null, new object[] { evt });
575575

576576
Assert.AreEqual(1, _tabView.SelectedIndex);
@@ -585,7 +585,7 @@ public void OnTabClicked_WithNonLabelTarget_DoesNothing()
585585
BindingFlags.NonPublic | BindingFlags.Static);
586586

587587
var evt = (MouseDownEvent)Activator.CreateInstance(typeof(MouseDownEvent), true);
588-
evt.target = new VisualElement();
588+
evt.currentTarget = new VisualElement();
589589
method.Invoke(null, new object[] { evt });
590590

591591
Assert.AreEqual(0, _tabView.SelectedIndex);
@@ -605,7 +605,7 @@ public void OnTabMouseEnter_InactiveTab_SetsHoverBackground()
605605
Assert.IsNotNull(method, "OnTabMouseEnter method should exist");
606606

607607
var evt = (MouseEnterEvent)Activator.CreateInstance(typeof(MouseEnterEvent), true);
608-
evt.target = inactiveTab;
608+
evt.currentTarget = inactiveTab;
609609
method.Invoke(_tabView, new object[] { evt });
610610

611611
Assert.AreEqual(Tokens.Colors.BgHover, inactiveTab.style.backgroundColor.value);
@@ -624,7 +624,7 @@ public void OnTabMouseEnter_ActiveTab_DoesNotChangeBackground()
624624
var method = typeof(JTabView).GetMethod("OnTabMouseEnter",
625625
BindingFlags.NonPublic | BindingFlags.Instance);
626626
var evt = (MouseEnterEvent)Activator.CreateInstance(typeof(MouseEnterEvent), true);
627-
evt.target = activeTab;
627+
evt.currentTarget = activeTab;
628628
method.Invoke(_tabView, new object[] { evt });
629629

630630
Assert.AreEqual(expectedBg, activeTab.style.backgroundColor.value);

UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Internal/BootstrapEditorUITests.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,13 @@ public void SafeFormat_EmptyTemplate_ReturnsEmpty()
677677
Assert.AreEqual("", result);
678678
}
679679

680+
[Test]
681+
public void SafeFormat_NullTemplate_ReturnsEmpty()
682+
{
683+
var result = BootstrapText.SafeFormat(null, "arg");
684+
Assert.AreEqual(string.Empty, result);
685+
}
686+
680687
#endregion
681688

682689
#region BootstrapText Default Tests

0 commit comments

Comments
 (0)