Make role ban pannel pretty (#37952)

* Make role ban pannel pretty

* Removed unused depencency

* refactor: wider panel (no jumping due to scroll in english lang) minor readability improvements

---------

Co-authored-by: pa.pecherskij <pa.pecherskij@interfax.ru>

Content.Client/Administration/UI/BanPanel/BanPanel.xaml

@@ -1,7 +1,7 @@
 <DefaultWindow
     xmlns="https://spacestation14.io"
     xmlns:cc="clr-namespace:Content.Client.Administration.UI.CustomControls"
-    Title="{Loc ban-panel-title}" MinSize="350 500">
+    Title="{Loc ban-panel-title}" MinSize="410 500">
     <BoxContainer Orientation="Vertical">
         <TabContainer Name="Tabs" VerticalExpand="True">
             <!-- Basic info -->

Content.Client/Administration/UI/BanPanel/BanPanel.xaml.cs

@@ -1,12 +1,14 @@
 using System.Linq;
 using System.Net;
 using System.Net.Sockets;
+using System.Numerics;
 using Content.Client.Administration.UI.CustomControls;
 using Content.Shared.Administration;
 using Content.Shared.CCVar;
 using Content.Shared.Database;
 using Content.Shared.Roles;
 using Robust.Client.AutoGenerated;
+using Robust.Client.GameObjects;
 using Robust.Client.Graphics;
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.Controls;
@@ -31,14 +33,21 @@ public sealed partial class BanPanel : DefaultWindow
     private uint Multiplier { get; set; }
     private bool HasBanFlag { get; set; }
     private TimeSpan? ButtonResetOn { get; set; }
+
     // This is less efficient than just holding a reference to the root control and enumerating children, but you
     // have to know how the controls are nested, which makes the code more complicated.
-    private readonly List<CheckBox> _roleCheckboxes = new();
+    // Role group name -> the role buttons themselves.
+    private readonly Dictionary<string, List<Button>> _roleCheckboxes = new();
     private readonly ISawmill _banpanelSawmill;
 
     [Dependency] private readonly IGameTiming _gameTiming = default!;
     [Dependency] private readonly IConfigurationManager _cfg = default!;
     [Dependency] private readonly ILogManager _logManager = default!;
+    [Dependency] private readonly IEntityManager _entMan = default!;
+    [Dependency] private readonly IPrototypeManager _protoMan = default!;
+
+    private const string ExpandedArrow = "▼";
+    private const string ContractedArrow = "▶";
 
     private enum TabNumbers
     {
@@ -144,47 +153,90 @@ public sealed partial class BanPanel : DefaultWindow
 
         ReasonTextEdit.Placeholder = new Rope.Leaf(Loc.GetString("ban-panel-reason"));
 
-        var prototypeManager = IoCManager.Resolve<IPrototypeManager>();
+        var departmentJobs = _protoMan.EnumeratePrototypes<DepartmentPrototype>()
-        foreach (var proto in prototypeManager.EnumeratePrototypes<DepartmentPrototype>())
+                                      .OrderBy(x => x.Weight);
+        foreach (var proto in departmentJobs)
         {
-            CreateRoleGroup(proto.ID, proto.Roles.Select(p =>  p.Id), proto.Color);
+            var roles = proto.Roles.Select(x => _protoMan.Index(x))
+                             .OrderBy(x => x.ID);
+            CreateRoleGroup(proto.ID, proto.Color, roles);
         }
 
-        CreateRoleGroup("Antagonist", prototypeManager.EnumeratePrototypes<AntagPrototype>().Select(p => p.ID), Color.Red);
+        var antagRoles = _protoMan.EnumeratePrototypes<AntagPrototype>()
+                                  .OrderBy(x => x.ID);
+        CreateRoleGroup("Antagonist", Color.Red, antagRoles);
     }
 
-    private void CreateRoleGroup(string roleName, IEnumerable<string> roleList, Color color)
+    /// <summary>
+    /// Creates a "Role group" which stores information and logic for one "group" of roll bans.
+    /// For example, all antags are one group, logi is a group, medical is a group, etc...
+    /// </summary>
+    private void CreateRoleGroup<T>(string groupName, Color color, IEnumerable<T> roles) where T : class, IPrototype
     {
         var outerContainer = new BoxContainer
         {
-            Name = $"{roleName}GroupOuterBox",
+            Name = $"{groupName}GroupOuterBox",
             HorizontalExpand = true,
             VerticalExpand = true,
             Orientation = BoxContainer.LayoutOrientation.Vertical,
-            Margin = new Thickness(4)
+            Margin = new Thickness(4),
         };
-        var departmentCheckbox = new CheckBox
+
+        // Stores stuff like ban all and expand buttons.
+        var roleGroupHeader = new BoxContainer
         {
-            Name = $"{roleName}GroupCheckbox",
+            Orientation = BoxContainer.LayoutOrientation.Horizontal,
-            Text = roleName,
-            Modulate = color,
-            HorizontalAlignment = HAlignment.Left
         };
-        outerContainer.AddChild(departmentCheckbox);
+
-        var innerContainer = new BoxContainer
+        // Stores the role checkboxes themselves.
+        var innerContainer = new GridContainer
         {
-            Name = $"{roleName}GroupInnerBox",
+            Name = $"{groupName}GroupInnerBox",
             HorizontalExpand = true,
-            Orientation = BoxContainer.LayoutOrientation.Horizontal
+            Columns = 2,
+            Visible = false,
+            Margin = new Thickness(15, 5, 0, 5),
         };
-        departmentCheckbox.OnToggled += args =>
+
+        var roleGroupCheckbox = CreateRoleGroupHeader(groupName, roleGroupHeader, color, innerContainer);
+
+        outerContainer.AddChild(roleGroupHeader);
+
+        // Add the roles themselves
+        foreach (var role in roles)
         {
-            foreach (var child in innerContainer.Children)
+            AddRoleCheckbox(groupName, role.ID, innerContainer, roleGroupCheckbox);
+        }
+
+        outerContainer.AddChild(innerContainer);
+
+        RolesContainer.AddChild(new PanelContainer
+        {
+            PanelOverride = new StyleBoxFlat
             {
-                if (child is CheckBox c)
+                BackgroundColor = color
-                {
+            }
-                    c.Pressed = args.Pressed;
+        });
-                }
+        RolesContainer.AddChild(outerContainer);
+        RolesContainer.AddChild(new HSeparator());
+    }
+
+    private Button CreateRoleGroupHeader(string groupName, BoxContainer header, Color color, GridContainer innerContainer)
+    {
+        var roleGroupCheckbox = new Button
+        {
+            Name = $"{groupName}GroupCheckbox",
+            Text = "Ban all",
+            Margin = new Thickness(0, 0, 5, 0),
+            ToggleMode = true,
+        };
+
+        // When this is toggled, toggle all buttons in this group so they match.
+        roleGroupCheckbox.OnToggled += args =>
+        {
+            foreach (var role in _roleCheckboxes[groupName])
+            {
+                role.Pressed = args.Pressed;
             }
 
             if (args.Pressed)
@@ -199,15 +251,12 @@ public sealed partial class BanPanel : DefaultWindow
             }
             else
             {
-                foreach (var childContainer in RolesContainer.Children)
+                foreach (var roleButtons in _roleCheckboxes.Values)
                 {
-                    if (childContainer is Container)
+                    foreach (var button in roleButtons)
                     {
-                        foreach (var child in childContainer.Children)
+                        if (button.Pressed)
-                        {
+                            return;
-                            if (child is CheckBox { Pressed: true })
-                                return;
-                        }
                     }
                 }
 
@@ -220,38 +269,72 @@ public sealed partial class BanPanel : DefaultWindow
                 SeverityOption.SelectId((int) newSeverity);
             }
         };
-        outerContainer.AddChild(innerContainer);
+
-        foreach (var role in roleList)
+        var hideButton = new Button
         {
-            AddRoleCheckbox(role, innerContainer, departmentCheckbox);
+            Text = Loc.GetString("role-bans-expand-roles") + " " + ContractedArrow,
-        }
+            ToggleMode = true,
-        RolesContainer.AddChild(new PanelContainer
+        };
+        hideButton.OnPressed += args =>
         {
-            PanelOverride = new StyleBoxFlat
+            innerContainer.Visible = args.Button.Pressed;
-            {
+            ((Button)args.Button).Text = args.Button.Pressed
-                BackgroundColor = color
+                ? Loc.GetString("role-bans-contract-roles") + " " + ExpandedArrow
-            }
+                : Loc.GetString("role-bans-expand-roles") + " " + ContractedArrow;
+        };
+        header.AddChild(new Label
+        {
+            Text = groupName,
+            Modulate = color,
+            Margin = new Thickness(0, 0, 5, 0),
         });
-        RolesContainer.AddChild(outerContainer);
+        header.AddChild(roleGroupCheckbox);
-        RolesContainer.AddChild(new HSeparator());
+        header.AddChild(hideButton);
+        return roleGroupCheckbox;
     }
 
-    private void AddRoleCheckbox(string role, Control container, CheckBox header)
+    /// <summary>
+    /// Adds a checkbutton specifically for one "role" in a "group"
+    /// E.g. it would add the Chief Medical Officer "role" into the "Medical" group.
+    /// </summary>
+    private void AddRoleCheckbox(string group, string role, GridContainer roleGroupInnerContainer, Button roleGroupCheckbox)
     {
-        var roleCheckbox = new CheckBox
+        var roleCheckboxContainer = new BoxContainer();
+        var roleCheckButton = new Button
         {
             Name = $"{role}RoleCheckbox",
-            Text = role
+            Text = role,
+            ToggleMode = true,
         };
-        roleCheckbox.OnToggled += args =>
+        roleCheckButton.OnToggled += args =>
         {
-            if (args is { Pressed: true, Button.Parent: { } } && args.Button.Parent.Children.Where(e => e is CheckBox).All(e => ((CheckBox) e).Pressed))
+            // Checks the role group checkbox if all the children are pressed
-                header.Pressed = args.Pressed;
+            if (args.Pressed && _roleCheckboxes[group].All(e => e.Pressed))
+                roleGroupCheckbox.Pressed = args.Pressed;
             else
-                header.Pressed = false;
+                roleGroupCheckbox.Pressed = false;
         };
-        container.AddChild(roleCheckbox);
+
-        _roleCheckboxes.Add(roleCheckbox);
+        // This is adding the icon before the role name
+        // Yeah, this is sus, but having to split the functions up and stuff is worse imo.
+        if (_protoMan.TryIndex<JobPrototype>(role, out var jobPrototype) && _protoMan.TryIndex(jobPrototype.Icon, out var iconProto))
+        {
+            var jobIconTexture = new TextureRect
+            {
+                Texture = _entMan.System<SpriteSystem>().Frame0(iconProto.Icon),
+                TextureScale = new Vector2(2.5f, 2.5f),
+                Stretch = TextureRect.StretchMode.KeepCentered,
+                Margin = new Thickness(5, 0, 0, 0),
+            };
+            roleCheckboxContainer.AddChild(jobIconTexture);
+        }
+
+        roleCheckboxContainer.AddChild(roleCheckButton);
+
+        roleGroupInnerContainer.AddChild(roleCheckboxContainer);
+
+        _roleCheckboxes.TryAdd(group, []);
+        _roleCheckboxes[group].Add(roleCheckButton);
     }
 
     public void UpdateBanFlag(bool newFlag)
@@ -469,7 +552,13 @@ public sealed partial class BanPanel : DefaultWindow
             if (_roleCheckboxes.Count == 0)
                 throw new DebugAssertException("RoleCheckboxes was empty");
 
-            rolesList.AddRange(_roleCheckboxes.Where(c => c is { Pressed: true, Text: { } }).Select(c => c.Text!));
+            foreach (var button in _roleCheckboxes.Values.SelectMany(departmentButtons => departmentButtons))
+            {
+                if (button is { Pressed: true, Text: not null })
+                {
+                    rolesList.Add(button.Text);
+                }
+            }
 
             if (rolesList.Count == 0)
             {

Resources/Locale/en-US/administration/ui/role-bans.ftl

@@ -0,0 +1,3 @@
+role-bans-ban-group = Ban All
+role-bans-expand-roles = Show Roles
+role-bans-contract-roles = Hide Roles