Page MenuHomePhabricator

Layout shift when IP Info arrives into expanded state (insufficient height reserved)
Closed, ResolvedPublic2 Estimated Story Points

Description

I noticed that panel on Special Contribs is hidden by default for client-nojs. This is great!

And the section also renders partly server-side to avoid FOUC and layout shifts when the default state is collapsed.

When the default state is expanded, however, it seems the placeholder is empty with little to no reserved height, even after the user preference is known and "loading" state has started. Thus, causing a layout shift not just in the initial rendering once the (cached) JS arrives, but also after the API response has come back from the network.

This shift is quite noticable and can lead to frustration due to accidental clicks or lost visual context.

Steps:

  1. https://en.wikipedia.beta.wmflabs.org/wiki/Special:RecentChanges?userExpLevel=unregistered
  2. Open devtools
  3. Pick a user and open their contribs page (if not expanded previously, expand once, and visit another contribs page)

Actual result:

Kapture 2022-04-23 at 20.33.22.gif (445×802 px, 591 KB)

AC:

  • The default height to be the same as when the infobox is open with data.

Related:

Event Timeline

Krinkle renamed this task from IP Info client reserves insufficient height to Layout shift when IP Info arrives into expanded state (insufficient height reserved).Apr 23 2022, 7:39 PM
Krinkle moved this task from Limbo to Perf recommendation on the Performance-Team (Radar) board.

Hi! @Prtksxna, would you like to provide your feedback and give us your opinion and directions for this ticket?

Thank you!

Thanks for flagging @AGueyte!

@Krinkle is the 'For 115.166,145.81 (talk…' part loading later too or is that because of some scrolling? Would keeping the height to include the loading animation be enough to solve this? I understand that once the content loads the height will shift again, I don't know if there is precedence to use animation for such a thing.

@Krinkle is the 'For 115.166,145.81 (talk…' part loading later too or is that because of some scrolling?

Neither, the "For (IP address)" portion is shifting downward due to the default CentralNotice banner active on beta wikis. This continous in beta, but much less common in production. It is tracked as its own issue, but I wouldn't worry about it in the context of IP Info.

I understand that once the content loads the height will shift again, I don't know if there is precedence to use animation for such a thing.

It is my understanding that animating the height in this context has no precedence. From past experience, I suspect we may want to avoid animating as it would prolong the delay before the information is ready to use, and prolong the perceived amount of time it takes for the information to "arrive" from an end-user perspective. I believe it would "feel" slower, and come across as involving more moving parts and higher cost, thus likely leading to reduced use due to the perception that it isn't instantly available. Given the expanded state is remembered, this might also turn the Contributions page more generally into an unpleasant experience as it would always be animating on load, not only providing a negative experience for "IP Info" but also for the Contributions page more generally by making it feel unready everytime it is used. Patrollers and curators frequently use this throughout their workflows, easily several times a minute.

It is also my current understanding that the IP Info content is rendered in a predictable layout and of finite and predictable length. If that understanding is correct, then it would be feasible with relatively ease to set the loading state such that in >99% of cases no shift would happen after the API responds with the data, and thus that the layout would actually not shift again when inserting the content values.

Hello @Krinkle,

Thank you both, @Krinkle and @Prtksxna for your answers.

The IPInfo box can appear under 4 different states, and 4 different heights.

  • When it shows an error banner
  • When the user needs to agree to the legal disclaimer
  • When it shows data
  • When it shows data but it's not available.

We could also have a situation where there's more data (proxies), and the list would be longer.
It seems we won't have one general height to set.

What do you both think?
Thank you!

Screen Shot 2022-05-10 at 9.22.53 AM.png (940×2 px, 207 KB)

Screen Shot 2022-05-10 at 9.28.49 AM.png (842×2 px, 177 KB)

Screen Shot 2022-05-10 at 9.23.49 AM.png (954×2 px, 226 KB)

Screen Shot 2022-05-10 at 9.30.30 AM.png (656×2 px, 118 KB)

Screen Shot 2022-05-10 at 9.25.26 AM.png (596×2 px, 94 KB)

Thanks @Krinkle! Yep makes sense to have it take up the prerequisite amount of space from the beginning. There have been some conversations around the "shimmering rectangle" loading animation that might make sense here, but that is a different task altogether (both for design systems and dev).


Thank you so much for laying this out @AGueyte, it was really helpful. I had some questions about the different states:

  • The disclaimer state:
    • When someone enables the beta feature for the first time, it starts off in the disclaimer state with the IP info box collapsed?
    • Are the contents of the disclaimer state loaded later causing a jump, or does it come pre-rendered?
  • When is this empty state (F35124372) shown? Is it another form of the error state (F35124373)?
  • If the tool is in use (ie disclaimer has been accepted) the two most common states would be with data (F35124376) or with data not available (F35124374).
    • Is this assumption correct?
    • Are the heights of these two states the same?
    • Are we able to reliably pre-calculate the height of this box across different languages?

Hi Prateek, thanks for your questions!

The disclaimer state:
When someone enables the beta feature for the first time, it starts off in the disclaimer state with the IP info box collapsed?

Yes

Are the contents of the disclaimer state loaded later causing a jump, or does it come pre-rendered?

It's pre-rendered

When is this empty state (F35124372) shown? Is it another form of the error state (F35124373)?

Yes, but this should be rare

If the tool is in use (ie disclaimer has been accepted) the two most common states would be with data (F35124376) or with data not available (F35124374).
Is this assumption correct?

Yes

Are the heights of these two states the same?

We do not have enough experience to assume this, since we have mainly been using Maxmind with test IPs.

Are we able to reliably pre-calculate the height of this box across different languages?

Hard to say at this point!

Thank you, I hope this helps!

@AGueyte would it be safe to say that all the headings + one line data would be a common use-case? Can we use that height to be the default height of the open box during the loading state (for those who have already accepted the T&C?

Even if the height changes later based on the data it'll be lesser than in the case of the smaller loading state.

AGueyte set the point value for this task to 2.Jun 27 2022, 5:27 PM

Change 812438 had a related patch set uploaded (by TsepoThoabala; author: TsepoThoabala):

[mediawiki/extensions/IPInfo@master] Layout shift when IP Info arrives into expanded state insufficient height reserved

https://gerrit.wikimedia.org/r/812438

Thanks for the patch @TThoabala - I think we should move ahead with it, but just noting a caveat here.

We do still have a jump where the infobox is collapsed until the JS loads. This is because the expanded/collapsed state is stored in local storage (https://gerrit.wikimedia.org/g/mediawiki/extensions/IPInfo/+/e8cb64991f23ed124b70556e7cfb1335fcec2668/modules/ext.ipInfo/infobox/init.js#38).

This was changed from a user preference in T304430. The user preference did allow the state to be known up front (https://gerrit.wikimedia.org/r/c/mediawiki/extensions/IPInfo/+/777786/4/src/HookHandler/InfoboxHandler.php).

I don't think there's much we can do except use the user preference again, but presumably we shouldn't do that for the reasons in T304430.

Change 812438 merged by jenkins-bot:

[mediawiki/extensions/IPInfo@master] Layout shift when IP Info arrives into expanded state insufficient height reserved

https://gerrit.wikimedia.org/r/812438

The loading animation is now not the same size as the infobox while it is loading. @Prtksxna is this ok?

infobox_expanding.gif (768×1 px, 1 MB)

Thanks for the screen capture, Dom!

The loading animation is now not the same size as the infobox while it is loading. @Prtksxna is this ok?

It is ok but not ideal. Is this something we can fix? If so, happy to file a follow up task for that.


Two follow-up questions:

  • The jump between the infobox collapsed and expanded happens fairly quickly, right? I understand that we have it this way for reasons (T304430) and it is only a minor annoyance so I think its okay.
  • In Dom's screen cap I noticed that there is a slight jump when the data loads, even though there isn't anything particularly different about the loaded data (no multiline data for eg). Is this because once the data is loaded the height is determined by default fonts etc, things that we can't really predict?

The loading animation is now not the same size as the infobox while it is loading. @Prtksxna is this ok?

It is ok but not ideal. Is this something we can fix? If so, happy to file a follow up task for that.

I am not sure. @TThoabala will know more.

Two follow-up questions:

  • The jump between the infobox collapsed and expanded happens fairly quickly, right? I understand that we have it this way for reasons (T304430) and it is only a minor annoyance so I think its okay.

Opening the infobox? Yes, depending on how quick the user's browser is.

  • In Dom's screen cap I noticed that there is a slight jump when the data loads, even though there isn't anything particularly different about the loaded data (no multiline data for eg). Is this because once the data is loaded the height is determined by default fonts etc, things that we can't really predict?

That's a good point. I guess to some extent. For example, we cannot know whether the data the API returns will be multiline (as you point out). But, perhaps we can assume that it is most likely there will only be one line of data for each field, and preset the height based on that. Again, @TThoabala will know more.

Change 819592 had a related patch set uploaded (by TsepoThoabala; author: TsepoThoabala):

[mediawiki/extensions/IPInfo@master] Layout shift when IP Info arrives into expanded state

https://gerrit.wikimedia.org/r/819592

The loading animation is now not the same size as the infobox while it is loading. @Prtksxna is this ok?
It is ok but not ideal. Is this something we can fix? If so, happy to file a follow up task for that.

I think its something we can fix and I will be happy to include that work in this ticket, see screenshot below

loading.png (922×2 px, 254 KB)

In Dom's screen cap I noticed that there is a slight jump when the data loads, even though there isn't anything particularly different about the loaded data (no multiline data for eg). Is this because once the data is loaded the height is determined by default fonts etc, things that we can't really predict?

I have created a follow up patch on this I didn't test on firefox before and it needed a bit of adjustment on min-height.

@Prtksxna and @dom_walden for your attention please...

Change 819592 merged by jenkins-bot:

[mediawiki/extensions/IPInfo@master] Layout shift when IP Info arrives into expanded state

https://gerrit.wikimedia.org/r/819592

I have created a follow up patch on this I didn't test on firefox before and it needed a bit of adjustment on min-height.

The patch I was referring to here got merged and it didn't include the animation change.
I have filed a task for it. https://phabricator.wikimedia.org/T314394

Moving to QA since there's a separate task for the product/design question. Thanks @TThoabala !

The loading animation is now not the same size as the infobox while it is loading. @Prtksxna is this ok?
It is ok but not ideal. Is this something we can fix? If so, happy to file a follow up task for that.

I think its something we can fix and I will be happy to include that work in this ticket, see screenshot below

loading.png (922×2 px, 254 KB)

@Prtksxna has given the go ahead on this (thank you), so I've moved T314394 to our triage column.

I didn't see a jump when the data loads on Vector 2022, Vector 2010 and Minerva, but only for single-line data.

There is a slight jump for MonoBook and a more notable one for Timeless (see below). @Prtksxna are we OK to optimise for Vector?

timeless_infobox_jump.gif (768×1 px, 1019 KB)

Thanks Dom! Yep makes sense to optimize for Vector (2022 > 2010, in case they're different) 👍🏾

Thanks Dom! Yep makes sense to optimize for Vector (2022 > 2010, in case they're different) 👍🏾

Thanks.

In that case, there is nothing more for me to do here.

Test browsers: Firefox 91, Chromium 87, Safari 15.