Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/ssr full display all invisibly #41

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

huan-qiu
Copy link

背景 & 复现方法:
close react-component/menu#665

期许:
在overflow ssr="full" 模式下, 在首次渲染所有 items 内容时, SEO友好且页面不闪烁

代码改动简述:

逻辑部分
在 ssr="full", shouldResponsive 为true时,
让首次 render 时会绘制所有 items(display prop 为true, css 宽度为0);
message channel 执行更新 containerWidth itemWidths 等值(值虽有, 但都为0), 触发第二次 render

第2次 render 时所有 items(但display prop 为false, css 宽度则回归正常), 随后触发 layoutEffect, layoutEffect 更新 displayCount 为 data.length - 1, 同步开始第3次 render, render所有的 items 并绘制(此次display prop 为false, css 宽度正常)

随后等到 message channel 再次执行更新 itemWidths 等值, 触发第4次 render, 此时 itemWidths 赋有正确的值, 因此所有 items(display prop 为 true, css 宽度正常), 后触发 layoutEffect, 更新 displayCount 为正确的计算结果. 同步开始第5次render, 并正确绘制出应该可见的 items

更新 ssr 相关测试

@vercel
Copy link

vercel bot commented Oct 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
overflow ❌ Failed (Inspect) Oct 14, 2023 1:54am

let suffixStyle: React.CSSProperties = {};
const isFullySSRResponsiveFirstRender =
fullySSR && shouldResponsive && containerWidth === null;
const fullySSRFirstRenderStyle: React.CSSProperties = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

有点蛋疼,效果感觉也不好。SSR 的时候这块就直接空了也会有 CLS。如果是 SSR Full Render 然后 SSR 的时候 overflow hidden 掉呢?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在overflow没设置高度下,会有一点shift,因为高度被撑大了。如果有设置的话,就是先空白然后显示,layout上就不会shift。如果是 SSR Full Render + overflow hidden,假设能让 item 是都显示在同一行溢出的话,效果是后面两个item的位置可能会闪一下。这个的前提是要 overflow 这个组件不要 wrap 成多行,目前是会wrap成多行,需要先有高度限制然后来 x y 都overflow hidden。看似都有 tradeoff。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zombieJ overflow组件 有接受个style的prop,因此在pr改动的基础上,可以传入height的值来先占好需要的高度。我理解这样做在 fullySSR 就不会有 CLS 的问题。因为 fullySSR 这个问题目前只会影响到 overflow 是水平一行 maxCount 值是RESPONSIVE的时候。看了下 rc-menu 使用 rc-overflow的时候是会传入 style prop的,rc-select 则是没开启 ssr,所以没有这个问题。你康康ok不?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zombieJ 如果暂时没有满意的方案,可以先让ssr=full在 menu 那里可以 opt out 吗?即考虑这条 pr react-component/menu#665

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #41 (3e9f5b2) into master (0553047) will decrease coverage by 1.18%.
The diff coverage is 83.33%.

❗ Current head 3e9f5b2 differs from pull request most recent head 411fb3b. Consider uploading reports for the commit 411fb3b to get more accurate results

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
- Coverage   98.10%   96.92%   -1.18%     
==========================================
  Files           6        6              
  Lines         211      228      +17     
  Branches       72       86      +14     
==========================================
+ Hits          207      221      +14     
- Misses          4        7       +3     
Files Coverage Δ
src/Overflow.tsx 97.53% <83.33%> (-1.78%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants