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/seo display all invisibly #40

Closed

Conversation

huan-qiu
Copy link

@huan-qiu huan-qiu commented Sep 27, 2023

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

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

代码改动简述:

  1. 逻辑部分

在 maxCount === "responsive" 时,
让首次 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

  1. 添加相关测试cases

@vercel
Copy link

vercel bot commented Sep 27, 2023

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

Name Status Preview Comments Updated (UTC)
overflow ❌ Failed (Inspect) Sep 27, 2023 2:38am

@yoyo837
Copy link
Member

yoyo837 commented Sep 27, 2023

文件为啥改名?

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #40 (bb0f020) into master (0553047) will decrease coverage by 0.34%.
The diff coverage is 90.47%.

❗ Current head bb0f020 differs from pull request most recent head cbf13ad. Consider uploading reports for the commit cbf13ad to get more accurate results

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
- Coverage   98.10%   97.76%   -0.34%     
==========================================
  Files           6        6              
  Lines         211      224      +13     
  Branches       72       85      +13     
==========================================
+ Hits          207      219      +12     
- Misses          4        5       +1     
Files Coverage Δ
src/Overflow.tsx 98.73% <90.47%> (-0.58%) ⬇️

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

@huan-qiu
Copy link
Author

huan-qiu commented Sep 27, 2023

文件为啥改名?

之前大神zombieJ 说 ssr 是为了SEO, rc-menu 默认是 responsive ssr 为 "full"
我想着 SEO 是个好东西, 如果 responsive 在 ssr undefined 的时候, 如果可以实现原本效果且逻辑和full统一的话, 好像可以试一下.
所以就统一在一起, ssr这个prop就不需要. 对应的测试文件就改了名字.

@@ -39,9 +39,6 @@ export interface OverflowProps<ItemType> extends React.HTMLAttributes<any> {

/** @private This API may be refactor since not well design */
onVisibleChange?: (visibleCount: number) => void;

/** When set to `full`, ssr will render full items by default and remove at client side */
ssr?: 'full';
Copy link
Member

Choose a reason for hiding this comment

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

这个不该删的,只是处理 full 的时候做坍缩。不是 full 的时候仍然是不渲染。overflow 不仅仅是 menu 在用。还有其他的响应时需求逻辑是不一样的。

Copy link
Author

Choose a reason for hiding this comment

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

猴猴,我缩小了改动的区域,在 full 且需要responsive的时候才做坍缩。麻烦 review 这条哦:#41

@huan-qiu huan-qiu closed this Oct 17, 2023
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

3 participants