-
Notifications
You must be signed in to change notification settings - Fork 16
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
ПРОЧИТАТЬ МЕРЕД МЕРДЖОМ [UPSTREAM] #159
Changes from 149 commits
14e0c3f
1507ff2
6ad7f7e
52d7863
8be60b0
a07cb3b
26b36db
318b052
c8bd723
917c6e2
37955f5
d0f4226
5790f5d
f55ae3f
d9b3859
e520731
d1c682b
9fb6c66
66d9012
3282045
fb05ea9
0da43eb
9a08d91
7055c05
4fdf2f5
92e837c
1d38ee6
4ab7975
7e55241
dc57afd
0e2bc19
571b132
3ea547b
facb6c0
557c3a9
f8a1cc5
a13d423
0dbbe65
a42395e
2080df5
54bf615
2c1a061
32a9dd4
935562c
8117ce5
41a70be
77c5f01
bde3c04
aad9b24
027df56
7c0e5aa
7f608b6
6604ec6
4cc0fca
6cb9037
70ee6f3
03f9649
a4cac04
9ecf70b
a0beaf4
fd93054
8c56c60
4f00a3c
04a33dc
20cb202
ecc378e
37ee718
ab7a8f0
95ea9e2
7c74034
958808c
c138f8d
26201a1
fbdc55e
60f3949
868297b
6780806
bdc0a52
2698e89
a2c5a83
cc23335
2b6243b
764602a
86b893f
c623405
203ec26
24cbd06
1e1b47f
7add23d
190030e
6098390
fd429b4
dabe904
461549c
bd23e0f
6ced034
13c1e01
88b47d2
746b6c8
b5ee0f4
53ebfe8
28f16b6
16c08f8
3a1c59e
3fe460c
5d7e6bd
8829c54
140fc65
2c65179
31964b6
9e510f0
77f96e7
dd6675d
86e6782
1227893
2bf19cf
e1efe46
9c3e4f7
9db6651
cb8df8c
34f47cb
21dd83c
dacfbb7
fbbb2ad
019cc24
45ab87a
3b9fa30
6879e50
9ba78c5
d7e3c04
2b0b875
485ecc5
50f87d4
9bef33b
17443d6
3279b3f
f28dfeb
a7678e3
d41c00a
0ffb2c2
956eb51
a1a26c3
3a7fc17
49d6731
f66f410
d8c0cfd
296c728
437a9d6
453be48
56c810d
e0eb768
d6b5a88
0f9b47c
07f57f8
bcc4ea6
b66fc91
2e1d6f3
abda95e
9bad455
086b40a
7d727a7
4e21385
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,13 @@ protected override void Open() | |
_window.OnNameChanged += OnNameChanged; | ||
_window.OnJobChanged += OnJobChanged; | ||
_window.OnJobIconChanged += OnJobIconChanged; | ||
_window.OnNumberChanged += OnNumberChanged; // DeltaV | ||
} | ||
|
||
// DeltaV - Add number change handler | ||
private void OnNumberChanged(uint newNumber) | ||
{ | ||
SendMessage(new AgentIDCardNumberChangedMessage(newNumber)); | ||
} | ||
Comment on lines
+32
to
36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add input validation for number changes The OnNumberChanged handler sends the new number directly without validation. Consider adding bounds checking or format validation. private void OnNumberChanged(uint newNumber)
{
+ // Add validation for acceptable number ranges
+ if (newNumber < MIN_NUMBER || newNumber > MAX_NUMBER)
+ {
+ Logger.Warning($"Invalid number value: {newNumber}");
+ return;
+ }
SendMessage(new AgentIDCardNumberChangedMessage(newNumber));
}
|
||
|
||
private void OnNameChanged(string newName) | ||
|
@@ -56,6 +63,7 @@ protected override void UpdateState(BoundUserInterfaceState state) | |
_window.SetCurrentName(cast.CurrentName); | ||
_window.SetCurrentJob(cast.CurrentJob); | ||
_window.SetAllowedIcons(cast.CurrentJobIconId); | ||
_window.SetCurrentNumber(cast.CurrentNumber); // DeltaV | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,6 +6,10 @@ | |||||
<LineEdit Name="NameLineEdit" /> | ||||||
<Label Name="CurrentJob" Text="{Loc 'agent-id-card-current-job'}" /> | ||||||
<LineEdit Name="JobLineEdit" /> | ||||||
<!-- DeltaV - Add NanoChat number field --> | ||||||
<Label Name="CurrentNumber" Text="{Loc 'agent-id-card-current-number'}" /> | ||||||
<LineEdit Name="NumberLineEdit" PlaceHolder="#0000" /> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding input validation constraints The NumberLineEdit control should have input validation to ensure only valid numbers are entered. Consider adding:
-<LineEdit Name="NumberLineEdit" PlaceHolder="#0000" />
+<LineEdit Name="NumberLineEdit" PlaceHolder="#0000" MaxLength="4" HorizontalAlignment="Left" MinWidth="60" /> 📝 Committable suggestion
Suggested change
|
||||||
<!-- DeltaV end --> | ||||||
<Label Text="{Loc 'agent-id-card-job-icon-label'}"/> | ||||||
<GridContainer Name="IconGrid" Columns="10"> | ||||||
<!-- Job icon buttons are generated in the code --> | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,9 +21,13 @@ public sealed partial class AgentIDCardWindow : DefaultWindow | |
|
||
private const int JobIconColumnCount = 10; | ||
|
||
private const int MaxNumberLength = 4; // DeltaV - Same as NewChatPopup | ||
|
||
public event Action<string>? OnNameChanged; | ||
public event Action<string>? OnJobChanged; | ||
|
||
public event Action<uint>? OnNumberChanged; // DeltaV - Add event for number changes | ||
|
||
public event Action<ProtoId<JobIconPrototype>>? OnJobIconChanged; | ||
|
||
public AgentIDCardWindow() | ||
|
@@ -37,6 +41,37 @@ public AgentIDCardWindow() | |
|
||
JobLineEdit.OnTextEntered += e => OnJobChanged?.Invoke(e.Text); | ||
JobLineEdit.OnFocusExit += e => OnJobChanged?.Invoke(e.Text); | ||
|
||
// DeltaV - Add handlers for number changes | ||
NumberLineEdit.OnTextEntered += OnNumberEntered; | ||
NumberLineEdit.OnFocusExit += OnNumberEntered; | ||
|
||
// DeltaV - Filter to only allow digits | ||
NumberLineEdit.OnTextChanged += args => | ||
{ | ||
if (args.Text.Length > MaxNumberLength) | ||
{ | ||
NumberLineEdit.Text = args.Text[..MaxNumberLength]; | ||
} | ||
|
||
// Filter to digits only | ||
var newText = string.Concat(args.Text.Where(char.IsDigit)); | ||
if (newText != args.Text) | ||
NumberLineEdit.Text = newText; | ||
}; | ||
} | ||
|
||
// DeltaV - Add number validation and event | ||
private void OnNumberEntered(LineEdit.LineEditEventArgs args) | ||
{ | ||
if (uint.TryParse(args.Text, out var number) && number > 0) | ||
OnNumberChanged?.Invoke(number); | ||
Comment on lines
+67
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error feedback for invalid input The validation silently ignores invalid input. Consider adding user feedback for better UX. - if (uint.TryParse(args.Text, out var number) && number > 0)
- OnNumberChanged?.Invoke(number);
+ if (uint.TryParse(args.Text, out var number))
+ {
+ if (number > 0)
+ OnNumberChanged?.Invoke(number);
+ else
+ // TODO: Inject IUserInterfaceManager for showing popups
+ Logger.Warning($"Invalid number input: {args.Text} (must be greater than 0)");
+ }
|
||
} | ||
|
||
// DeltaV - Add setter for current number | ||
public void SetCurrentNumber(uint? number) | ||
{ | ||
NumberLineEdit.Text = number?.ToString("D4") ?? ""; | ||
Comment on lines
+71
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add input validation for SetCurrentNumber The method should validate the input number to ensure consistency with the UI constraints. public void SetCurrentNumber(uint? number)
{
+ if (number != null && (number.ToString().Length > MaxNumberLength || number == 0))
+ {
+ Logger.Error($"Invalid number value: {number}");
+ return;
+ }
NumberLineEdit.Text = number?.ToString("D4") ?? "";
}
|
||
} | ||
|
||
public void SetAllowedIcons(string currentJobIconId) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
<BoxContainer | ||
xmlns="https://spacestation14.io" | ||
xmlns:graphics="clr-namespace:Robust.Client.Graphics;assembly=Robust.Client" | ||
HorizontalExpand="True"> | ||
<Button Name="ChatButton" | ||
StyleClasses="ButtonSquare" | ||
HorizontalExpand="True" | ||
MaxSize="137 64" | ||
Margin="0 1"> | ||
<BoxContainer Orientation="Horizontal" | ||
HorizontalExpand="True" | ||
VerticalExpand="True" | ||
MinWidth="132" | ||
Margin="6 4" | ||
VerticalAlignment="Center"> | ||
<!-- Unread indicator dot --> | ||
<PanelContainer Name="UnreadIndicator" | ||
MinSize="8 8" | ||
MaxSize="8 8" | ||
VerticalAlignment="Center" | ||
Margin="0 0 6 0"> | ||
<PanelContainer.PanelOverride> | ||
<graphics:StyleBoxFlat | ||
BackgroundColor="#17c622" | ||
BorderColor="#0f7a15" /> | ||
</PanelContainer.PanelOverride> | ||
</PanelContainer> | ||
|
||
<!-- Text container --> | ||
<BoxContainer Orientation="Vertical" | ||
HorizontalExpand="True" | ||
VerticalExpand="True" | ||
VerticalAlignment="Center"> | ||
<RichTextLabel Name="NameLabel" | ||
StyleClasses="LabelHeading" | ||
HorizontalExpand="True" | ||
HorizontalAlignment="Center" | ||
VerticalAlignment="Center" | ||
Margin="0 -2 0 0" /> | ||
Comment on lines
+34
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Based on the search results, I can now provide a final response. The codebase shows consistent patterns for handling text in UI components: Add ClipText property to handle text overflow The RichTextLabel should have
<RichTextLabel Name="NameLabel"
StyleClasses="LabelHeading"
HorizontalExpand="True"
HorizontalAlignment="Center"
VerticalAlignment="Center"
ClipText="True"
Margin="0 -2 0 0" /> 🔗 Analysis chainVerify text overflow handling for long names The RichTextLabel for names doesn't have explicit text clipping or wrapping configuration. This could lead to layout issues with very long names. The previous search failed because the file extension wasn't recognized. Let's try a different approach by using 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for any existing text overflow handling patterns in similar UI components
rg -A 2 "ClipText=\"True\"" --type xaml
Length of output: 68 Script: #!/bin/bash
# Search for any existing text overflow handling patterns in similar UI components
# Using a more general search since the file extension might be different
fd -e xaml . | xargs cat | grep -A 2 'ClipText='
# Also search for RichTextLabel usage patterns
fd -e xaml . | xargs cat | grep -A 2 'RichTextLabel'
# Search for potential style classes that might handle text overflow
rg -A 2 "LabelHeading"
Length of output: 34797 |
||
<Label Name="JobLabel" | ||
StyleClasses="LabelSubText" | ||
HorizontalExpand="True" | ||
ClipText="False" | ||
HorizontalAlignment="Center" /> | ||
</BoxContainer> | ||
</BoxContainer> | ||
</Button> | ||
</BoxContainer> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
using Content.Shared.DeltaV.CartridgeLoader.Cartridges; | ||
using Robust.Client.AutoGenerated; | ||
using Robust.Client.UserInterface.Controls; | ||
using Robust.Client.UserInterface.XAML; | ||
|
||
namespace Content.Client.DeltaV.CartridgeLoader.Cartridges; | ||
|
||
[GenerateTypedNameReferences] | ||
public sealed partial class NanoChatEntry : BoxContainer | ||
{ | ||
public event Action<uint>? OnPressed; | ||
private uint _number; | ||
private Action<EventArgs>? _pressHandler; | ||
|
||
public NanoChatEntry() | ||
{ | ||
RobustXamlLoader.Load(this); | ||
} | ||
|
||
public void SetRecipient(NanoChatRecipient recipient, uint number, bool isSelected) | ||
{ | ||
// Remove old handler if it exists | ||
if (_pressHandler != null) | ||
ChatButton.OnPressed -= _pressHandler; | ||
|
||
_number = number; | ||
|
||
// Create and store new handler | ||
_pressHandler = _ => OnPressed?.Invoke(_number); | ||
ChatButton.OnPressed += _pressHandler; | ||
|
||
NameLabel.Text = recipient.Name; | ||
JobLabel.Text = recipient.JobTitle ?? ""; | ||
JobLabel.Visible = !string.IsNullOrEmpty(recipient.JobTitle); | ||
UnreadIndicator.Visible = recipient.HasUnread; | ||
|
||
ChatButton.ModulateSelfOverride = isSelected ? NanoChatMessageBubble.OwnMessageColor : null; | ||
} | ||
Comment on lines
+20
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add parameter validation and cleanup Several improvements needed for robustness:
+public sealed partial class NanoChatEntry : BoxContainer, IDisposable
{
public event Action<uint>? OnPressed;
private uint _number;
private Action<EventArgs>? _pressHandler;
public void SetRecipient(NanoChatRecipient recipient, uint number, bool isSelected)
{
+ ArgumentNullException.ThrowIfNull(recipient);
+
// Remove old handler if it exists
if (_pressHandler != null)
ChatButton.OnPressed -= _pressHandler;
_number = number;
// Create and store new handler
- _pressHandler = _ => OnPressed?.Invoke(_number);
+ _pressHandler = static (_, n) => OnPressed?.Invoke(n);
ChatButton.OnPressed += _pressHandler;
NameLabel.Text = recipient.Name;
JobLabel.Text = recipient.JobTitle ?? "";
JobLabel.Visible = !string.IsNullOrEmpty(recipient.JobTitle);
UnreadIndicator.Visible = recipient.HasUnread;
ChatButton.ModulateSelfOverride = isSelected ? NanoChatMessageBubble.OwnMessageColor : null;
}
+
+ public void Dispose()
+ {
+ if (_pressHandler != null)
+ {
+ ChatButton.OnPressed -= _pressHandler;
+ _pressHandler = null;
+ }
+ }
}
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<BoxContainer xmlns="https://spacestation14.io" | ||
xmlns:customControls="clr-namespace:Content.Client.Administration.UI.CustomControls" | ||
Margin="4" | ||
Orientation="Vertical"> | ||
<BoxContainer Orientation="Horizontal"> | ||
<Label Name="NumberLabel" | ||
Align="Right" | ||
SetWidth="26" | ||
ClipText="True" /> | ||
<Label Name="TimeLabel" | ||
Align="Center" | ||
SetWidth="100" | ||
ClipText="True" /> | ||
<Label Name="MessageLabel" | ||
Align="Left" | ||
MinWidth="390" | ||
HorizontalExpand="True" | ||
ClipText="False" /> | ||
</BoxContainer> | ||
<customControls:HSeparator Margin="0 5 0 5" /> | ||
</BoxContainer> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,17 @@ | ||||||||||||||||||||||||||||||||||||
using Robust.Client.AutoGenerated; | ||||||||||||||||||||||||||||||||||||
using Robust.Client.UserInterface.Controls; | ||||||||||||||||||||||||||||||||||||
using Robust.Client.UserInterface.XAML; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
namespace Content.Client.DeltaV.CartridgeLoader.Cartridges; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
[GenerateTypedNameReferences] | ||||||||||||||||||||||||||||||||||||
public sealed partial class NanoChatLogEntry : BoxContainer | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
public NanoChatLogEntry(int number, string time, string message) | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
RobustXamlLoader.Load(this); | ||||||||||||||||||||||||||||||||||||
NumberLabel.Text = number.ToString(); | ||||||||||||||||||||||||||||||||||||
TimeLabel.Text = time; | ||||||||||||||||||||||||||||||||||||
MessageLabel.Text = message; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+10
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add parameter validation and error handling The constructor should include parameter validation to ensure robustness:
Consider applying this improvement: public NanoChatLogEntry(int number, string time, string message)
{
+ ArgumentNullException.ThrowIfNull(time);
+ ArgumentNullException.ThrowIfNull(message);
+
RobustXamlLoader.Load(this);
- NumberLabel.Text = number.ToString();
+ NumberLabel.Text = number.ToString(CultureInfo.InvariantCulture);
TimeLabel.Text = time;
MessageLabel.Text = message;
} Don't forget to add: +using System;
+using System.Globalization; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
} |
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.
Add event handler cleanup
The number change event subscription should be cleaned up when the interface is closed.