hlxsites / prisma-cloud-docs-website

blocks and gdoc authored content for https://docs.prismacloud.io
Apache License 2.0
3 stars 2 forks source link

Large layout shifts when switching between docs #73

Closed charlieboles closed 1 year ago

charlieboles commented 1 year ago

Observation of the Issue/Behavior There are large content shifts and a page shake / jump when navigating between product documentation. This makes for a disjointed experience and makes it harder for the user to re-orient themselves when moving between documentation.

Expected Result/Behavior Page should not jump / shake when navigating to a new doc and text should not jump. Ex: Page/text does not shake or jump when navigating between different docs here: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/create-service-console-v2.html

Steps to Reproduce

  1. Navigate to https://main--prisma-cloud-docs-website--hlxsites.hlx.live/prisma/prisma-cloud/en/compute/public-sector/disa-stig/disa-stig
  2. Click on Release Findings in left nav
  3. Notice large page shift
  4. Click on Isolated upgrades in left nav
  5. Notice large page shift

Devices Used macOS 13.3.1

Browsers Used Chrome v114

https://github.com/hlxsites/prisma-cloud-docs-website/assets/2379680/0b41f4b9-90b9-4b09-a797-f4edd325ee60

maxakuru commented 1 year ago

@charlieboles could you expand on this

what shift are you referring to? difference of content/styles between pages or the CLS/FOUC that occurs during render? or the fact that some of those articles don't have enough content to push the footer below the fold?

icaraps commented 1 year ago

@maxakuru I believe this is happening because of the breadcrumbs which is pushing the article content down by min 20px. Here's a recording without breadcrumbs. AFAICS there's no layout shift in this case compared to the recording above.

https://github.com/hlxsites/prisma-cloud-docs-website/assets/6012307/11f9bb34-eb5f-452c-97cd-2301ab2fcf00

I can think of 2 ways to solve this issue:

  1. Currently the breadcrumbs is loaded after the article, so if we load the breadcrumbs before the article it should work fine but then I don't know what's going to be the impact on performance.
  2. Reserve some space for the breadcrumbs but the problem is depending how many items are in the breadcrumbs, it can wrap and therefore you don't really know how much space to reserve.

wdyt?

maxakuru commented 1 year ago

@icaraps I don't get a huge layout shift for either case tbh, but I imagine you're right the breadcrumbs cause most of it..

  1. the problem with loading breadcrumbs first is that most of the items rely on the data in book.json, which ends up being pretty expensive, especially pre-LCP

  2. I actually tried this a bit ago and found the same problem: we don't know how much to reserve. I gave up on it since it's just as bad (maybe worse) to have the space too large then shrink on render. Localization make this especially hard since we can't rely on pathname segment lengths for estimates of breadcrumb sizes either..

imo the best solution involves a change to the design, just by truncating breadcrumbs to a single line and only showing the last N entries that will fit, but that might not be an option

@iansk / @charlieboles could you confirm that the CLS you're talking about here is breadcrumbs? any thoughts on this, especially considering there's a new design that just went live?

iansk commented 1 year ago

@maxakuru @icaraps I have a meeting with @charlieboles early tomorrow morning. I'll get clarification on this issue, and report back here.

icaraps commented 1 year ago

I don't get a huge layout shift for either case tbh, but I imagine you're right the breadcrumbs cause most of it..

@maxakuru It's more noticeable with slowed down network settings for example with fast 3G network:

https://github.com/hlxsites/prisma-cloud-docs-website/assets/6012307/be915268-04fc-4c80-ac9c-e5c3373dd59c

Since you mentioned the footer, you might also notice the footer flashing in at the beginning of the recording.

charlieboles commented 1 year ago

@maxakuru @icaraps

Thank you for looking into this. I can confirm that this issue is not limited to the breadcrumbs, there seems to be some type of race condition with loading that causes noticeable CLS that is not seen on other documentation sites. We're all developers here and have had to move through our share of documentation if there is consistent CLS that increases the cognitive load for the user as your eyes have to constantly reshift focus.

I've created more screen recordings and captured still frames. Hopefully, this helps with diagnosing the root cause. For reference, the internet connection used during this test was ~75mbps.

example-1-screencap

https://github.com/hlxsites/prisma-cloud-docs-website/assets/2379680/2393b740-09dc-4623-8b8e-426acee9e0f2

Example 2 where the footer is loaded before any doc content.

example-2-screencap

https://github.com/hlxsites/prisma-cloud-docs-website/assets/2379680/6316b865-2520-406c-a965-c44f3949218d

iansk commented 1 year ago

@icaraps Thanks for posting the video. Yes, that's exactly what @charlieboles is reporting. I just got off a call with him. He'll post more details here, but essentially, here's what's contributing to the shifting/jumpiness:

All of these types of things add a cognitive load to customers just trying to get in and read our docs. We'd like the site to feel robust and stable, without shifting around on each page loads.

AWS docs do a pretty good job of that type of stability (example link)

maxakuru commented 1 year ago

@iansk / @charlieboles it seems the issue here isn't so much about performance, but more out of concern for user experience when the user clicks around on a topic they're reading.. is that right?

CLS is one thing in LH, and another in actual use.. since the site is currently using links to new pages, there's always going to be some level of flash while the page repaints. The example above for AWS docs is an SPA so it doesn't have the same issue.

I added SPA rendering for the sidenav links to test out whether it's what you're after, you can try it out here: https://spa-nav--prisma-cloud-docs-website--hlxsites.hlx.page/prisma/prisma-cloud/en/compute/pcee/admin-guide/agentless-scanning/onboard-accounts/onboard-aws

note that it's not ready, certain things won't update yet, but it will give you the rough idea .. lmk thoughts

iansk commented 1 year ago

@maxakuru This is fantastic! The SPA side nav substantially improves the experience. Once the page is fully loaded, moving through the links feels sturdy/robust/high-quality.

My only remaining comment is the initial page load. Things are still bouncing around. This is what I noticed in my browser:

I realize we need to start talking about constraints/trade-offs to make things work the way we want. You previously mentioned truncating breadcrumbs to a single line and only showing the last N entries that will fit. I'm certainly open to this suggestion, and any others.

icaraps commented 1 year ago

Here's an attempt to solve the breadcrumbs and footer load issue https://github.com/hlxsites/prisma-cloud-docs-website/pull/97

maxakuru commented 1 year ago

@iansk / @charlieboles - just merged @icaraps changes and the spa loading, take a look again and see what you think

charlieboles commented 1 year ago

@maxakuru & @icaraps This is looking great! I did notice one thing though.

On the linked book-layout-shift PR. When scrolling and navigating to a new page the user is brought back to the top of the page. This is the expected / desired outcome.

However, it looks like this PR was merged into main and no longer has this behavior. On route change the scroll position is maintained so users are starting half way down the page on a new doc. Is it possible to bring back the functionality on the book-layout-shift branch by always starting the user at the top of the page on route change?

iansk commented 1 year ago

@maxakuru @icaraps Quick follow-up. Can you please fix this last scrolling issue today? I'd like to get it fixed ASAP so Charlie can start his work.

maxakuru commented 1 year ago

@iansk yup, closed in #108