Skip to content

Mod tools interface: Temporary pull request for feedback only #2040

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.

Sign up for GitHub

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

Closed
trichoplax wants to merge 13 commits into from
Closed

Mod tools interface: Temporary pull request for feedback only #2040

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f16bd3a
Temporary commit to confirm what should be cherry picked from the mod…
trichoplax Apr 23, 2026
6c29d6d
Add missing image upload modal for warnings
trichoplax Apr 23, 2026
ed3e5d9
Formatting improvements thanks to Rubocop
trichoplax Apr 23, 2026
bcfedf9
Make 'without_sidebar' the default layout for the users controller
trichoplax Apr 24, 2026
588c019
Add missing do block (thanks Rubocop!)
trichoplax Apr 24, 2026
c2acb3d
Remove expand buttons in mod warning log
trichoplax Apr 27, 2026
ae78b0b
Restore formatting of mod tools previous warning log
trichoplax Apr 27, 2026
efd7158
Make user mod tools left panel move to top on mobile
trichoplax May 15, 2026
126c2c4
Only render once in impersonate method
trichoplax May 15, 2026
586c9c9
Avoid separate views for desktop and mobile mod tools lists
trichoplax May 24, 2026
6ad16ae
Improve code formatting
trichoplax May 24, 2026
7c87bb1
Match left panel layout change in mod tools to right panel in other p…
trichoplax May 26, 2026
8de70f0
Add temporary CSS variable for mod tools
trichoplax May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/assets/javascripts/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// This is a manifest file that'll be compiled into application.js, which will include all the files
// listed below.
//
Expand Down Expand Up @@ -37,6 +37,15 @@
dialog.classList.toggle('is-active');
});

QPixel.DOM.addSelectorListener('click', '.is-partial-only:not(.open)', (ev) => {
if (ev.target.classList.contains("open")) {
return;
}

ev.target.classList.add("open");
ev.stopPropagation();
});

if (document.cookie.indexOf('dismiss_fvn') === -1) {
QPixel.DOM.addSelectorListener('click', '#fvn-dismiss', (_ev) => {
document.cookie = 'dismiss_fvn=true; path=/; expires=Fri, 31 Dec 9999 23:59:59 GMT';
Expand Down
31 changes: 31 additions & 0 deletions app/assets/stylesheets/application.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,3 +265,34 @@ kbd {
right: 0.15em;
};
}

.is-partial-only:not(.open) {
max-height: 100px;
overflow: hidden;
position: relative;

&::after {
content: '';
position: absolute;
right: 0; left: 0;
bottom: 0;
height: 75px;
background-color: rgba(255, 255, 255, 0.5);
background: linear-gradient(#ffffff66, #fffffffa);
z-index: 10000000;
}

&::before {
content: 'expand';
position: absolute;
left: 50%;
transform: translate(-50%);
bottom: 10px;
z-index: 10000001;
padding: 5px 20px;
background-color: #ddd;
border: 1px solid #666;
border-radius: 15px;
cursor: pointer;
}
}
6 changes: 5 additions & 1 deletion app/assets/stylesheets/tabs.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@

.tabs {
margin-bottom: 1em;
}

.tabs--push {
flex-grow: 1;
}
}
15 changes: 15 additions & 0 deletions app/assets/stylesheets/users.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,3 +241,18 @@ $sizes: (16, 32, 40, 48, 64, 128, 256);
}
}
}

.modtools--sidebar {
margin-right: 1rem;
}
.modtools--usercard {
padding: 0.5rem;
}
.modtools-tbl-noborder {
th {
border-bottom-width: 1px !important;
width: 150px;
}
}

.mod-warnings-clear-form { display: inline; }
6 changes: 3 additions & 3 deletions app/controllers/admin_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
class AdminController < ApplicationController
before_action :verify_admin, except: [:change_back, :verify_elevation]
before_action :verify_global_admin, only: [:admin_email, :send_admin_email, :new_site, :create_site, :setup,
:setup_save, :hellban, :all_email, :send_all_email]
:setup_save, :failban, :all_email, :send_all_email]
before_action :verify_developer, only: [:change_users, :impersonate]
before_action :set_user, only: [:change_users, :hellban, :impersonate]
before_action :set_user, only: [:change_users, :failban, :impersonate]

skip_before_action :check_if_warning_or_suspension_pending, only: [:change_back, :verify_elevation]

Expand Down Expand Up @@ -204,7 +204,7 @@ def setup_save
render
end

def hellban
def failban
@user.block("user manually blocked by admin ##{current_user.id}")
flash[:success] = t 'admin.user_fed_stat'
redirect_back fallback_location: admin_path
Expand Down
1 change: 1 addition & 0 deletions app/controllers/moderator_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def user_vote_summary
total: Vote.for(@user).count
)
)
render layout: 'without_sidebar'
end

def spammy_users
Expand Down
28 changes: 22 additions & 6 deletions app/controllers/users_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ class UsersController < ApplicationController

before_action :redirect_to_sign_in, only: [:filters], unless: [:user_signed_in?, :json_request?]

before_action :verify_moderator, only: [:mod, :destroy, :soft_delete, :role_toggle, :full_log,
:annotate, :annotations, :mod_privileges, :mod_privilege_action]
before_action :set_user, only: [:show, :mod, :destroy, :soft_delete, :posts, :role_toggle, :full_log, :activity,
:annotate, :annotations, :mod_privileges, :mod_privilege_action,
:vote_summary, :network, :avatar]
before_action :verify_moderator, only: [:mod, :soft_delete, :role_toggle, :full_log,
:annotate, :annotations, :mod_privileges, :mod_privilege_action, :mod_delete]
before_action :verify_global_moderator, only: [:mod_failban, :mod_delete_network_account]
before_action :set_user, only: [:show, :mod, :mod_delete, :mod_failban, :mod_delete_network_account, :soft_delete,
:posts, :role_toggle, :full_log, :activity, :annotate, :annotations, :mod_privileges,
:mod_privilege_action, :vote_summary, :network, :avatar]
before_action :check_deleted, only: [:show, :posts, :activity]

def index
Expand Down Expand Up @@ -275,7 +276,9 @@ def activity
render layout: 'without_sidebar'
end

def mod; end
def mod
render layout: 'without_sidebar'

@ArtOfCode- ArtOfCode- Apr 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's a lot of these in this controller - is it worth just making it the default layout for the controller?

@trichoplax trichoplax Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wasn't aware of how to do that so I've had a look into it.

There's the standard Rails way of doing it, by adding layout "without_sidebar" at the top of the controller file. I noticed we also have a variable @prevent_sidebar that is used in places where the sidebar is to be omitted only sometimes.

The advantage of a variable is that it can be made to only affect that one aspect. Currently it does not. Currently the variable is used to choose between the default application.html.erb and without_sidebar.html.erb. Unfortunately the two layout files differ in more than just whether the sidebar is included. I don't know if this is due to accidental divergence due to having near identical code in 2 separate places, or whether there is currently a need for the other differences.

If there is a need for the other differences with the pages that currently happen to have no sidebar, this is not clear from the name so it would be easy for someone to add or remove a sidebar on a page that should not have the other differences.

Even if there is currently no accidental divergence, the fact that I had to look into this to understand that there are more differences suggests it would be easy for someone with my level of knowledge to accidentally cause divergence in future by modifying either of these layouts without the other.

How would you feel about this being raised as a separate issue to consider how best to mitigate these risks? I can imagine a solution that involves no longer having a without_sidebar layout, and just having the sidebar in application.html.erb be controlled by a variable, so there are no surprising side effects.

I'm happy to also make the change here in the meantime if you prefer. In case it isn't obvious, I was out of my depth with this question so these are just my thoughts based on finding out how this currently works.

@ArtOfCode- ArtOfCode- Apr 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can imagine a solution that involves no longer having a without_sidebar layout, and just having the sidebar in application.html.erb be controlled by a variable, so there are no surprising side effects.

That sounds ideal to me. For now let's use the layout call here to avoid widening the scope of this work again, but we should file it for future - it's not difficult, but will be a chunk of work to convert the layout and change all existing layout calls to use that approach.

@trichoplax trichoplax Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've now applied 'without_sidebar' as the default layout for the Users controller, and removed this layout call from affected methods. There were only a couple that needed to be overridden back to the 'application' layout, so this has made a big difference - thanks. I've completely removed those methods that existed only for this reason.

I've also raised #2043 for the future work.

end

def full_log
@posts = Post.by(@user).count
Expand Down Expand Up @@ -319,8 +322,21 @@ def full_log
render layout: 'without_sidebar'
end

def mod_delete_network_account
render layout: 'without_sidebar'
end

def mod_failban
render layout: 'without_sidebar'
end

def mod_privileges
@abilities = Ability.all
render layout: 'without_sidebar'
end

def mod_delete
render layout: 'without_sidebar'
end

def soft_delete
Expand Down
89 changes: 48 additions & 41 deletions app/views/mod_warning/log.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,49 @@
<h1>Warnings sent to <%= user_link @user %></h1>
<% content_for :title, "Moderator Tools: #{rtl_safe_username(@user)}" %>

<table class="table is-full-width is-with-hover">
<tr>
<th>Date</th>
<th>Type</th>
<th>From</th>
<th>Excerpt</th>
<th>Status</th>
</tr>
<% @warnings.each do |w| %>
<tr>
<td>
<span title="<%= w.created_at.iso8601 %>"><%= time_ago_in_words(w.created_at) %> ago</span>
</td>
<td>
<% if w.suspension? %>
<% diff = ((w.suspension_end - w.created_at) / (3600 * 24)).to_i %>
<span class="badge is-tag is-red is-filled nowrap">Suspension</span> (<%= diff %>d)
<% else %>
<span class="badge is-tag is-muted nowrap">Warning</span>
<% end %>
</td>
<td><%= user_link w.author %></td>
<td><%= raw(sanitize(render_markdown(w.body), scrubber: scrubber)) %></td>
<td class="wrap-word">
<% if w.suspension_active? %>
<strong>Current</strong>
<%= form_tag lift_mod_warning_url(@user.id), method: :post do %>
<%= submit_tag '(lift)', class: 'link is-red' %>
<% end %>
<% elsif w.active %>
<strong>Unread</strong>
<% elsif w.read %>
Read
<% else %>
<strong>Lifted</strong>
<% end %>
</td>
</tr>
<% end %>
</table>
<%= render 'users/tabs', user: @user %>

<div class="grid">
<%= render 'shared/user_mod_sidebar', user: @user %>

<div class="grid--cell is-9">
<h1>Previously Sent Warnings</h1>

<% if @warnings.size == 0 %>
<p class="has-color-tertiary"><em>No warnings found for this user.</em></p>
<% end %>

<% @warnings.each do |w| %>
<div class="widget">
<div class="widget--header is-complex h-bg-tertiary-050">
<div class="has-float-right h-p-1 h-fs-caption">
<% if w.suspension_active? %>
<%= form_tag lift_mod_warning_url(@user.id), method: :post, class: 'mod-warnings-clear-form' do %>
<%= submit_tag '(lift)', class: 'link is-red' %>
<% end %> &middot;
<strong>Current <i class="fas fa-fw fa-clock"></i></strong>
<% elsif w.active %>
<strong>Unread <i class="fas fa-fw fa-eye-slash"></i></strong>
<% elsif w.read %>
Read <i class="fas fa-fw fa-eye"></i>
<% else %>
Lifted <i class="fas fa-fw fa-undo"></i>
<% end %>
</div>
<div>
<% if w.is_suspension %>
<% diff = ((w.suspension_end - w.created_at) / (3600 * 24)).to_i %>
<span class="badge is-tag is-red is-filled badge-nobreak"><i class="fas fa-user-slash fa-fw"></i> Suspension</span>
<span class="badge is-tag is-muted badge-nobreak">length: <%= diff %>d</span>
<% else %>
<span class="badge is-tag is-yellow is-filled badge-nobreak"><i class="fas fa-exclamation-triangle fa-fw"></i> Warning</span>
<% end %>
</div>
<div class="h-c-tertiary-700 h-fs-caption h-m-t-2"><span title="<%= w.created_at.iso8601 %>"><%= time_ago_in_words(w.created_at) %> ago</span> by <%= user_link w.author %></div>
</div>
<div class="widget--body is-partial-only">
<%= raw(sanitize(render_markdown(w.body), scrubber: scrubber)) %>
</div>
</div>
<% end %>
</div>
</div>
Loading
Loading