-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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: layout and Page
style adjustment
#5033
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/@core/ui-kit/layout-ui/src/vben-layout.vueOops! Something went wrong! :( ESLint: 9.16.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThe pull request introduces several modifications to the layout components, primarily focusing on enhancing responsiveness and styling. In Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
packages/effects/common-ui/src/components/page/page.vue (1)
103-105
: ReplacesetTimeout
withnextTick
for reactive updatesUsing
setTimeout
may introduce unnecessary delays and can be less reliable across different environments.nextTick
is more suitable for waiting until the DOM updates have been applied.Modify the
calcContentHeight
function:setTimeout(() => { shouldAutoHeight.value = true; - }, 30); + });Or use
nextTick
:await nextTick(); shouldAutoHeight.value = true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/@core/ui-kit/layout-ui/src/vben-layout.vue
(1 hunks)packages/effects/common-ui/src/components/page/page.vue
(5 hunks)playground/src/views/examples/vxe-table/basic.vue
(1 hunks)
🔇 Additional comments (5)
packages/effects/common-ui/src/components/page/page.vue (2)
Line range hint 140-161
: Optimize slot usage within header
Wrapping the title
and description
slots within a div
with class "flex-auto"
might impact the layout if not handled carefully. Ensure that this additional wrapper does not introduce unwanted side effects.
Please verify that the new wrapper maintains the intended layout and styling, especially in responsive scenarios.
83-93
:
Prevent content overlap with fixed footer
When the footer is fixed, the content may overlap with it if no margin or padding is added to the content area. To ensure a seamless layout, adjust the content area to account for the footer height.
Modify the contentStyle
to include paddingBottom
when the footer is fixed:
if (footerHeight.value > 0 && preferences.footer.fixed) {
style.marginBottom = `${footerHeight.value}px`;
+ style.paddingBottom = `${footerHeight.value}px`;
}
Likely invalid or redundant comment.
playground/src/views/examples/vxe-table/basic.vue (1)
74-74
: Appropriate use of fixed-header
prop in <Page>
component
Adding the fixed-header
prop to the <Page>
component enables the fixed header feature as intended. This aligns with the recent updates to the Page
component and addresses the issues related to header positioning.
packages/@core/ui-kit/layout-ui/src/vben-layout.vue (2)
506-506
: Adding overflow-hidden
class improves layout stability
Restoring the overflow-hidden
class to the main content div
addresses the table width calculation issues and prevents unwanted scrollbars from appearing. This change fixes the layout problems outlined in the PR objectives.
506-506
: Verify impact of overflow-hidden
on child components
While overflow-hidden
resolves certain layout issues, it may inadvertently hide overflowing content from child components like modals or dropdown menus. Ensure that this change does not negatively affect components that rely on overflow visibility.
Please test interactive components that may extend outside their parent container to confirm they display correctly.
444335a
to
08e2744
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
docs/src/components/layout-ui/page.md (2)
21-29
: Enhance props documentation for better clarityWhile the props table is well-structured, consider the following improvements:
- Expand the
fixedHeader
description to explicitly mention its relationship withheader.mode
, similar to how it's described in the note below.- Consider adding examples or default values for
headerClass
andfooterClass
to guide users on common usage patterns.| fixedHeader | 固定头部在页面内容区域顶部,在滚动时保持可见 | `boolean` | `false` | `v5.5.0` | +| fixedHeader | 固定头部在页面内容区域顶部,在滚动时保持可见(仅在 header.mode 为 fixed 时生效) | `boolean` | `false` | `v5.5.0` |
33-33
: Add layout considerations to the documentationGiven the layout-related fixes in this PR, consider expanding the note to include:
- A reference to the header mode documentation
- Common layout considerations when using
fixedHeader
- Potential impact on table layouts and scrolling behavior
-如果`title`、`description`、`extra`三者均未提供有效内容(通过`props`或者`slots`均可),则页面头部区域不会渲染。另外,`fixedHeader`属性目前仅在顶栏模式(`header.mode`)为`固定`(`fixed`)时有效。 +如果`title`、`description`、`extra`三者均未提供有效内容(通过`props`或者`slots`均可),则页面头部区域不会渲染。另外,`fixedHeader`属性目前仅在顶栏模式(`header.mode`)为`固定`(`fixed`)时有效。启用`fixedHeader`时,请注意: + +1. 确保父容器具有明确的高度定义 +2. 表格布局可能需要额外的滚动容器配置 +3. 树形表格可能需要特殊处理以维持正确的高度
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
docs/src/components/layout-ui/page.md
(1 hunks)packages/@core/ui-kit/layout-ui/src/vben-layout.vue
(1 hunks)packages/effects/common-ui/src/components/page/page.vue
(5 hunks)playground/src/views/examples/vxe-table/basic.vue
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- playground/src/views/examples/vxe-table/basic.vue
- packages/effects/common-ui/src/components/page/page.vue
- packages/@core/ui-kit/layout-ui/src/vben-layout.vue
这个问题我解决了,我将packages\effects\common-ui\src\components\page\page.vue内calcContentHeight方法改为了以下代码 const renderContent = ref(false); async function calcContentHeight() { 然后使用去控制default插槽渲染,解决了这个问题 |
Description
修复表格在窗口尺寸变小时,自动调整尺寸会变得非常缓慢的BUG;
修复
Page
组件的footer
没有固定在底部的问题;修复表格自定义列宽度超出页面宽度时,滚动条出现在页面而不是表格内的问题;
页面容器div的overflow-hidden移除之后会产生表格宽度计算问题,但添加overflow-hidden又会造成page组件内的header无法使用sticky定位。此PR恢复了页面容器的overflow-hidden类,将page组件的header和footer定位修改为fixed
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
New Features
fixed-header
property to improve table header display in the basic example.Bug Fixes
Documentation
Page
component to include versioning for props and clarify the functionality of thefixedHeader
property.