Given that the edit summary could in some cases could load very fast (less than 100ms) we don't want a case where we flash the animation for a very small amount of time. I am taking some inspiration from the [[ https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/font-display | font-display ]] browser behavior here.

1. Don't show any animation for 100ms and if we have the edit summary till then fade it in.
2. If don't have anything in 100ms, fade in the animation.
3. Once we receive the data check if the animation has been playing for at least 500ms
   
   A. If it has (been playing for 500ms), fade it out and switch it with the edit summary (like in the codepen)
   
   B. If not, wait so that the animation has played for at least 500ms (since the animation started, not since we got the data), and then switch it with the edit summary.

If we do this we are giving the user the data a bit late in one case, but by doing that we are avoiding visually jarring jumps.

Does this make sense for this case? What do you think @ifried @MusikAnimal?

In my opinion it's probably not worth adding all that specialized, potentially fragile logic for something as simple as a loading indicator. I think regardless you're going to get some jarring jumps, because we don't know how long the edit summary is, or if there is one at all. In all likelihood once it is displayed the size of the popup will change and appear to jump.

I would either always show a loading indicator, or none at all. I think no loading indicator may be acceptable; Loading indicators are of course great practice, but in general I don't think the user would get confused or upset if the edit summary suddenly appeared given they already have the important information they wanted, which is the author/timestamp and link to the diff.

I would agree with Leon here. Simplicity seems a smart tactic. Personally, I'd always show the loading indicator even if it disappears as soon as it's there. In my opinion, that's bomb-proof for those requests that might take a long time and much simpler than trying to guess how long something is going to take.

I agree that it's better to show the loading animation (rather than no animation at all). We anticipate times when the edit summary may take extra time to load. In such cases, it may be helpful to know that more data is coming, from a user perspective.

However, I think Leon made a great point about opting for simplicity. The option presented by @aezell ("always show the loading indicator even if it disappears as soon as it's there") may be easier to implement, although it's less responsive to changes. From a design perspective, what do you think, @Prtksxna?

In T231115#5476249, @aezell wrote:

I would agree with Leon here. Simplicity seems a smart tactic. Personally, I'd always show the loading indicator even if it disappears as soon as it's there. In my opinion, that's bomb-proof for those requests that might take a long time and much simpler than trying to guess how long something is going to take.

That was the original idea for the task (and codepen). During the estimation of the task I got the impression that this isn't ideal, maybe I misunderstood 🤔 As long as we know what the concerns were and and make sure that they're addressed I am happy to go back to the original 😊

Background: We originally discussed adding loading animation when the edit summary is present in the revision details pop-up. This is because the edit summary may sometimes take a bit of extra time to load. Then, in a meeting with the team, we discussed the fact that: a) sometimes the edit summary will load slowly (so loading animation is desirable), and b) sometimes the edit summary will load quickly (so loading animation may be unnecessary or jarring). At that time, we thought we could think of a way to accommodate both scenarios.

Now, there is the open question of whether we should use the loading animation --and, if so, when? Do we always show it? Do we show it under certain circumstances?

My thoughts: We should use the loading animation, so that users remain informed regarding additional data. I also understand that the animation may be jarring in certain circumstances. However, we simply do not know until we test out the loading behavior. For this reason, I propose that we first implement the basic version of the loading animation (i.e. loading animation as default step in the process, regardless of loading speed). However, if we do find that such behavior is jarring or awkward to users in certain scenarios, we can pursue the plan outlined by Prateek (which is sensical to me).

PR https://github.com/wikimedia/WhoWroteThat/pull/54, with a question for @Prtksxna

The PR is merged but it appears to me like this might be one of the things that the Mozilla folks were concerned about. The documentation even says, "Accepts raw HTML."

While we might feel confident that the API we control will give us back safe content, we should code more defensively. The Mozilla folks will insist on it and it's best practice generally.

Could we add this to the list/task where we are tracking the HTML sanitization work?

@MusikAnimal @ifried If the edit has no edit summary, do we still want to show the bytes difference? We did before

Before (from commit 8f1a1335dd644fad4e6abb57e25fae0a0d1fd716):

After:

@dom_walden Great question, and thanks for reming me of this issue!

First, the background: The original mock-ups for WWT included bytes in the revision details pop-up. However, after some discussion, we decided that bytes weren't necessary (so we opted to exclude them from requirements). Unfortunately, we didn't update the mock-ups when we added them to T228941 (which was the ticket to add in the edit summary). There was only a "Note" in the ticket, which stated that bytes don't need to be included. As a result, the edit summary was implemented with the bytes included (since it was easy to overlook the note). By the time this was noticed, Leon reached out in the ticket and asked if we should remove it. I thought it didn't hurt to have the byes if the work was already complete, so we kept them... but I wondered if the bytes should eventually be decoupled, which was the last comment on the edit summary ticket.

This leaves us with the following: bytes are only shown when edit summary is shown. So, this could be frustrating to users, since it's not standardized. We may want to standardize the view, by always or never showing bytes. What do you think, @Prtksxna?

A loading animation now appears in the revision details popup, regardless of whether there is an edit summary associated with the revision.

Similarly to T226746#5606568, I ran a script to compare the edit summary in the popup to what is returned by the MediaWiki API, for each token in a few different articles on enwiki, dewiki and eswiki. It did not detect any (non-spurious) differences.

In T231115#5600678, @ifried wrote:

...I wondered if the bytes should eventually be decoupled, which was the last comment on the edit summary ticket.

Cool.