Mod tools interface: Temporary pull request for feedback only

[trichoplax]

Please do not merge this pull request.

This includes only the user interface changes from #767 for feedback before I cherry pick the relevant commits to make a branch with identical outcome to this one but correct attribution of work done by other people.

This branch contains a bug that it turned out was already present in the develop branch so I have raised it separately as #2039 to keep this change small. There's also #2041 that I don't have access to confirm in production, but affects the develop branch in my local development environment.

I have endeavoured to remove anything not required for a solely visual change, so that all new functionality can be left for a future pull request. Please let me know if I've missed anything, or if anything does not work as expected, or if any existing functionality is missing here.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.96%. Comparing base (d694b97) to head (ae78b0b).

Additional details and impacted files

Components	Coverage Δ
controllers	76.24% <100.00%> (+0.05%)	⬆️
helpers	85.37% <ø> (ø)
jobs	66.88% <ø> (ø)
models	93.04% <ø> (ø)
tasks	61.11% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ArtOfCode-]

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

[trichoplax]

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-]

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]

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.

[cellio]

Things I noticed that we should come back to later and should absolutely not block this work moving forward:

• Dashboard: intro text should probably have the yellow-box styling, like on the "warn or suspend" page.
• Dashboard warnings/annotations section: would be nice to include the time of the most recent one, so a mod can tell at a glance if this is current or ancient (or somewhere in between).
• User annotations: this page, uniquely, is wide and can require horizontal scrolling. Another page that could do that, but doesn't, is the full activity log. Is there a difference in applied styles, or is it because of the code rendering on the annotation text?
• Warn or suspend user: currently begins with this warning box: "Use the warning tool only against users who have violated the site rules. Prefer other measures where possible, such as a public comment." First, "against" sounds confrontational; how about "with"? More substantially, there are other reasons to contact a user privately; a public comment can be too public sometimes. Let's wordsmith this somehow.
• Warn or suspend user: if the user is currently suspended, we probably want to indicate that on this page so a mod doesn't accidentally suspend a suspended user.
• Delete account (the community-specific one, not network): let's come back to the messaging in the text here; it's not aligned with current practice. (Ok, maybe we do need to give this one higher priority, but it should also be a simple change after you assemble the real PR.)

I'll file issues for these later and link them to the real PR as follow-on tasks, but the best time to notice them is on first viewing and I don't want to forget, so I'm parking this list here for now.

[cellio]

The UI changes look great -- I ran through the page as admin and as a regular mod and didn't run into any problems. (I noticed some smaller things to come back to later in a different comment; they were like that on the branch you're pulling from and they are not important enough to worry about now.)

I didn't review the code since the focus here is on preliminary testing/feedback.

[cellio]

One issue (should have caught this earlier): on a phone the individual views work pretty well (yay!), but the sidebar containing the links to the various tools isn't so great. We probably need a way to see all the tools , either as a slide-out or a separate page that's linked below a certain viewport width.

[trichoplax]

Prefer other measures where possible, such as a public comment

Yes it would be good to think of a better wording here. In the main mod tools branch the full wording is:

Prefer other measures where possible, such as a public comment or contacting them privately

The private comment option isn't available in this solely UI change pull request, so I've omitted the reference to contacting them privately (which in the main branch is a link to the page for sending a private comment).

[cellio]

The private comment option isn't available in this solely UI change pull request, so I've omitted the reference to contacting them privately (which in the main branch is a link to the page for sending a private comment).

Good point; this is an interim state until we get private conversations and non-warning contact, which are high priorities so will probably be among the earlier ones after the UI framing. We can definitely defer this.

[trichoplax]

the sidebar containing the links to the various tools isn't so great. We probably need a way to see all the tools , either as a slide-out or a separate page that's linked below a certain viewport width

Thanks for catching that. I should have thought to check how it looks on mobile.

This will need some decisions made, and definitely looks like something that will need to be addressed as part of this user interface branch - it doesn't maintain the existing functionality. In addition to finding a way to make the side panel still available when it can't be a side panel, there's also the fact that the username is no longer in the title of the various pages, because it's now at the top of the side panel. In a layout that has the side panel slide out or drop down from a hamburger menu, we'll probably need the username to be visible even when the side panel is hidden (to reduce the chance of applying a change or warning to the wrong user).

[trichoplax]

Your screenshot also shows the main thing I was already concerned about: Those expand buttons rather than just having a <details> section. Do you think they add anything that would justify making the page JavaScript dependent? Unlike <details> sections they cannot be collapsed once expanded, and won't work at all without JavaScript.

[cellio]

This will need some decisions made, and definitely looks like something that will need to be addressed as part of this user interface branch - it doesn't maintain the existing functionality. In addition to finding a way to make the side panel still available when it can't be a side panel, there's also the fact that the username is no longer in the title of the various pages, because it's now at the top of the side panel. In a layout that has the side panel slide out or drop down from a hamburger menu, we'll probably need the username to be visible even when the side panel is hidden (to reduce the chance of applying a change or warning to the wrong user).

How about if, at some suitable width threshold, we switch over to a one-column view, no sidebar, user card at the top with a link to a list of tools. The list would be just a list, not a sidebar; with luck it can share the same content as the sidebar but not be part of a two-section view. Cut-and-paste mockup of what I mean:

[cellio]

Your screenshot also shows the main thing I was already concerned about: Those expand buttons rather than just having a <details> section. Do you think they add anything that would justify making the page JavaScript dependent? Unlike <details> sections they cannot be collapsed once expanded, and won't work at all without JavaScript.

Personally, I would get rid of the JS dependency and the "expand" button, and either show the whole message or show the first, say, 500 characters of the message with a "more" link. Currently we show the whole message and I don't think we have message-specific links, but we presumably will when the "mod chat" feature is added, so instead of trying to make it fancy now, I would just show the whole message and revisit when we have something we could link to. I don't think we need fancy JS stuff that will break for people without JS.

[trichoplax]

How about if, at some suitable width threshold, we switch over to a one-column view, no sidebar, user card at the top with a link to a list of tools. The list would be just a list, not a sidebar; with luck it can share the same content as the sidebar but not be part of a two-section view

I like this. The sidebar is already pulled in from a separate view file, so that file can probably be used for both cases (sidebar on desktop and its own page on mobile). Maybe using a similar approach to the user profile, where the side panel appears or disappears as you change the window width or zoom level (just in this case it would convert to the link you describe, rather than moving to the bottom of the page).