23

Today, in the review queue, I stumbled upon this Suggested edit, which looks like this on my browser:

enter image description here

Looked simple enough to me: the editor removed a duplicated "is", that's clearly an improvement, and I don't see any obvious issues with the post (well, except that the whole first sentence is useless, but that's another debate), so I approved!
I then realized (after approving) that I must have missed something, because the edit was only 3 characters (and I think that there should be a 5 or 7-characters minimum length for suggested edits). So, I went back to it, opened the "Side-by-side Markdown" view, and, sure enough, the edit also changed the code. If I had seen that when reviewing, I would not have approved.

I realize that an answer could be "you should always check side-by-side markdown view", but the "side-by-side" view is in my opinion often more pleasant to view, and checking all views for every review just in case is not the most pleasant workflow either. And of course, reading the edit message would have alerted me that I was missing something, but for such a small edit, it's usually not necessary to check the edit message (and, in general, edit message are useless 95% of the time).

I'm not sure what would be the best way to represent this kind of changes, but something like a green/red mark on the side of the code block (or even before or after) would be useful in my opinion. Maybe like this:

enter image description here

Or at least some indication that part of the edit is hidden. Or it could be a warning saying "part of this edit modified code, which may not be visible without scrolling or using the side-by-side markdown view".

12
  • 6
    I mean, it had an edit summary
    – Kevin B
    Commented Sep 14, 2022 at 15:10
  • 4
    @KevinB one of the very few that say that. Or say anything really useful.
    – VLAZ
    Commented Sep 14, 2022 at 15:11
  • 6
    Markdown preview handles this better. Until this is implemented (if it is implemented, queues are back to being off the roadmaps), I suggest using markdown preview first, and checking the rendered output second Commented Sep 14, 2022 at 15:12
  • 4
    I generally always check Side-by-side markdown for this reason. Though I definitely agree that it would be nice if it were similarly evident in all views.
    – Henry Ecker Mod
    Commented Sep 14, 2022 at 15:12
  • 3
    I'm confused why this is an issue. For me, you can scroll to the right and see the changed code, even in the side-by-side view. Of course, I didn't see it in the edit queue. I clicked the link in the question. Maybe that makes a difference? Commented Sep 14, 2022 at 17:11
  • 3
    @GregBurghardt that assumes you know the code has changed. Reviewing all the code, particularly if there's a relative lot of it, to maybe find a change that may or may not have happened takes an unnecessary amount of time Commented Sep 14, 2022 at 22:12
  • "currentTarget.textContent" was changed to "target.innerText" Commented Sep 14, 2022 at 23:13
  • I thought the diff was highlighted when I last saw it (After scrolling the code block), but now even that seems to be missing. Seems to be the return of this bug. Commented Sep 15, 2022 at 4:53
  • There's a reasonable possibility for improvement here (i.e. have change markers to the left of each changed line in that view). However, the fact that you didn't see the change in the code is entirely on you. The change in the code is clearly visible and highlighted in all three views (Inline, Side-by-side, and Side-by-side Markdown). The issue is that you failed to scroll the code in order to see it. That's something that's necessary to do. If the post doesn't fit on the screen, including code, then you need to scroll the post, including code sections both horizontally and vertically.
    – Makyen Mod
    Commented Sep 15, 2022 at 15:11
  • 3
    @Makyen I agree that there was a reasonable way for me to see this change, and that, given the current implementation of the side-by-side view, it's "necessary" to check, as you say. However, that Zoe's comment "Reviewing all the code [...] to maybe find a change that may or may not have happened takes an unnecessary amount of time" explains the motivation behind this feature request. Everyday people complain that there are not enough reviewers. Well, making reviews easier to do (by not having to switch between views and scrolling through everything) is probably a good way to improve that.
    – Dada
    Commented Sep 15, 2022 at 15:22
  • If the goal is to not have to scroll to see the entire post unless there's a change that's not visible, then the views will need vertical change markers both for the entire post, as the change may be to long to fit on the current screen vertically, and vertical change markers within code sections, or any other independently scroll-able section (including things like MathJax).
    – Makyen Mod
    Commented Sep 15, 2022 at 15:37
  • The opposite happened to me here. At first it looked like it was just making useless edits to the text so I was about to reject it as "no improvement", but then I read the edit summary and realized that it also fixed a typo in the code, so I approved it. But it was very close that I missed the edit to the code which would have caused me to incorrectly reject it. Commented Dec 9, 2022 at 15:56

0

You must log in to answer this question.