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: opt out of ssr full to avoid initial flashing #665

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

Conversation

huan-qiu
Copy link

背景:
horizontal responsive模式下, 当全部 menu item 总宽度超出 overflow 宽度时, 默认全部 menu item 都会被绘制到页面, 出现溢出, 随后才会计算和隐藏.

期许:
rc-menu 可能仅在 client 端使用, 这个时候, 希望ssr 值非写死, 实现 menu item 当被绘制时是已经计算好的数量, 不出现目前的闪动.

复现方式:
可以用 antd menu 组件的第一个用例复现. 效果如下:

rc-menu-ssr-default-full-flash.mov

代码改动简述:
在不变动 rc-menu 默认 ssr 设置 “full“ 的前提下, 加入新prop来支持跳出该默认设置.

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #665 (5049d03) into master (89c09ad) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 5049d03 differs from pull request most recent head 9143b72. Consider uploading reports for the commit 9143b72 to get more accurate results

@@           Coverage Diff           @@
##           master     #665   +/-   ##
=======================================
  Coverage   99.57%   99.57%           
=======================================
  Files          27       27           
  Lines         713      714    +1     
  Branches      195      193    -2     
=======================================
+ Hits          710      711    +1     
  Misses          3        3           
Files Coverage Δ
src/Menu.tsx 98.66% <100.00%> (+<0.01%) ⬆️

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

@zombieJ
Copy link
Member

zombieJ commented Sep 26, 2023

直接裁剪的话,会导致 SEO 里没有东西。所以我特地加上了 SSR full 的。如果是这样,干脆 overflow 里加个逻辑,ssr 的时候渲染元素但是不占宽度,client 渲染在放出来? 

@huan-qiu
Copy link
Author

好哦, 这样的话 overflow 那里 display 计算逻辑可能要小改一下, 因为首次收集到的 menu items 的宽度数据会有误. 我关掉这条提个新的去 overflow?

@yoyo837
Copy link
Member

yoyo837 commented Sep 26, 2023

好哦, 这样的话 overflow 那里 display 计算逻辑可能要小改一下, 因为首次收集到的 menu items 的宽度数据会有误. 我关掉这条提个新的去 overflow?

可以直接改, 只要没有混合其他内容就行, 新开也可.

@huan-qiu
Copy link
Author

@zombieJ @yoyo837 刚提了条pr到overflow, 得空时候康康哦

@yoyo837
Copy link
Member

yoyo837 commented Sep 27, 2023

@zombieJ @yoyo837 刚提了条pr到overflow, 得空时候康康哦

当前PR还需要吗?

@huan-qiu
Copy link
Author

@zombieJ @yoyo837 刚提了条pr到overflow, 得空时候康康哦

当前PR还需要吗?

如果 take 了那条, 这条就不需要了

@huan-qiu
Copy link
Author

@yoyo837 如果这个pr ok可以先review这条不?另外那个后面再看

@yoyo837
Copy link
Member

yoyo837 commented Sep 30, 2023

@yoyo837 如果这个pr ok可以先review这条不?另外那个后面再看

如果那个可以代替这个, 那我建议直接CR那个, 目前假期中, 请稍等哈.

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.

3 participants