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(components): [tabs] optimize SSR #15183

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Conversation

makedopamine
Copy link
Collaborator

@makedopamine makedopamine commented Dec 17, 2023

Copy link

👋 @makedopamine, thank you for contributing element-plus.

  • You can comment with /label Components:[component_name] to add a label for which component you are working on.
  • You may join our Discord for staying tuned.

Copy link

github-actions bot commented Dec 17, 2023

Copy link

Hello @makedopamine, thank you for contributing to element-plus, please see our guideline to see how to make contribution

Copy link

github-actions bot commented Dec 17, 2023

🧪 Playground Preview: https://element-plus.run/?pr=15183
Please comment the example via this playground if needed.

@makedopamine makedopamine marked this pull request as draft December 17, 2023 15:48
@makedopamine makedopamine force-pushed the fix/tab_ssr branch 2 times, most recently from cb56200 to 437cc97 Compare December 18, 2023 10:29
@makedopamine makedopamine changed the title fix(components): [tabs] support SSR fix(components): [tabs] optimize SSR Dec 18, 2023
@makedopamine makedopamine marked this pull request as ready for review December 20, 2023 08:22
Copy link
Member

@tolking tolking left a comment

Choose a reason for hiding this comment

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

LGTM.

@qcyblm
Copy link

qcyblm commented May 12, 2024

@tolking Looking forward to seeing the update merge. Any idea when it will be pushed?

@btea
Copy link
Collaborator

btea commented Jul 27, 2024

There are code conflicts that need to be resolved.

@makedopamine
Copy link
Collaborator Author

There are code conflicts that need to be resolved.

Resolved

@btea btea removed the conflict pending need to resolve conflicts label Jul 29, 2024
@btea btea merged commit ae0439d into element-plus:dev Jul 29, 2024
9 checks passed
@makedopamine makedopamine deleted the fix/tab_ssr branch July 29, 2024 14:15
@element-bot element-bot mentioned this pull request Aug 9, 2024
3 tasks
? [header, panels]
: [panels, header]}
{panels}
{header}
Copy link
Contributor

Choose a reason for hiding this comment

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

tab-position value is top by default, that is, the header in the upper panels in the lower, why do we default the header in the lower, and then use flex-direction: column-reverse to go to the inverse again

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason had been mentioned in the above comment.

@zhixiaotong
Copy link
Contributor

@btea @tolking 这个pr导致了tabs的内容发生了改变,属于breaking change了。正常content在header下面,现在这个content在header上面了,然后又加了一个flex-direction: column-reverse;保证正常,但影响了原有的项目,升级新版本后,发现项目原有好多样式需要重写,影响蛮大的。

@tolking
Copy link
Member

tolking commented Sep 3, 2024

这个PR是属于breaking chang,所以才会在v2.8.0 合并。

对于普通用户来说影响应该挺小的。

@zhixiaotong
Copy link
Contributor

也许我们应该在changelog中体现这些变更的细节。
我习惯性在tabs下使用纵向flex布局,在升级之前一般会先看changlog,但问题发生才回溯代码发现是某些原因,之前也有几次升级出现过类似的问题。
我以前看过一些issue风评不好,这会导致用户在选择升级版本变得更加不确定。

@btea
Copy link
Collaborator

btea commented Sep 3, 2024

@zhixiaotong 多谢反馈,后续我们会尝试优化这方面的工作。

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

Successfully merging this pull request may close these issues.

[Component] [tabs, tab-pane] El-tabs not support SSR.
6 participants