-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat(components): Column settings support fixed columns. #874
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR adds support for fixed columns in the table column settings component, allowing users to pin columns to the left or right side of the table, or unfix them to allow normal scrolling.
Key Changes:
- Added
fixedproperty toTableColumnChecktype with values'left','right', or'unFixed' - Implemented UI controls in the column settings popover to cycle through fixed states
- Added localization support for fixed column labels in English and Chinese
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/typings/naive-ui.d.ts |
Added TableColumnFixed type definition extracted from TableColumnCheck['fixed'] |
src/typings/app.d.ts |
Added i18n schema for datatable fixed column labels (left, right, unFixed) |
src/locales/langs/zh-cn.ts |
Added Chinese translations for fixed column options |
src/locales/langs/en-us.ts |
Added English translations for fixed column options |
src/hooks/common/table.ts |
Updated getColumnChecks to initialize fixed property and getColumns to apply fixed state to columns |
packages/hooks/src/use-table.ts |
Added fixed property to TableColumnCheck type and updated reloadColumns to preserve fixed state |
src/components/advanced/table-column-setting.vue |
Added fixed column button with icon indicators and tooltip showing next state |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <icon-octicon-pin-16 v-if="item.fixed === 'unFixed'" /> | ||
| <icon-octicon-pin-16 v-else-if="item.fixed === 'left'" class="rotate-270" /> | ||
| <icon-octicon-pin-slash-16 v-else /> |
Copilot
AI
Dec 31, 2025
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.
The icon displayed for the 'right' fixed state is misleading. When a column is fixed to the right (item.fixed === 'right'), the code shows icon-octicon-pin-slash-16, which typically represents "unpinned" or "not fixed". This is confusing for users.
Consider using a more appropriate icon for the right-fixed state, such as:
- A pin icon rotated to point right (e.g.,
class="rotate-90") - Or keep the regular pin icon with a different styling to distinguish it from left-fixed
The current logic makes it appear that right-fixed columns are unpinned, which contradicts their actual behavior.
| .map(check => { | ||
| return { | ||
| ...columnMap.get(check.key), | ||
| fixed: check.fixed |
Copilot
AI
Dec 31, 2025
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.
The fixed property is being set to the string 'unFixed' which is not a valid value for naive-ui's DataTable columns. According to naive-ui's API, the fixed property should be either 'left', 'right', or undefined/false (not set).
When check.fixed is 'unFixed', the fixed property should not be set at all or should be set to undefined. Consider updating the logic to:
fixed: check.fixed !== 'unFixed' ? check.fixed : undefined
This ensures that columns marked as 'unFixed' don't have the fixed property set, which allows them to scroll normally.
| fixed: check.fixed | |
| fixed: check.fixed !== 'unFixed' ? check.fixed : undefined |
| ...col, | ||
| checked: checkMap.get(col.key) ?? col.checked | ||
| checked: checkMap.get(col.key) ?? col.checked, | ||
| fixed: (fixedMap.get(col.key) !== 'unFixed' ? fixedMap.get(col.key) : undefined) ?? col.fixed |
Copilot
AI
Dec 31, 2025
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.
The logic for preserving the fixed state has a bug. The current expression (fixedMap.get(col.key) !== 'unFixed' ? fixedMap.get(col.key) : undefined) ?? col.fixed will not correctly preserve a user's explicit choice to unfix a column.
When a user sets a column to 'unFixed', this logic evaluates to undefined ?? col.fixed, which falls back to the default col.fixed value. If the column was originally fixed (e.g., 'left' or 'right'), it will revert to that fixed state, ignoring the user's 'unFixed' preference.
The logic should be:
fixed: fixedMap.has(col.key) ? fixedMap.get(col.key) : col.fixed
This way, if a fixed preference exists in the map (including 'unFixed'), it's used; otherwise, fall back to the default.
| fixed: (fixedMap.get(col.key) !== 'unFixed' ? fixedMap.get(col.key) : undefined) ?? col.fixed | |
| fixed: fixedMap.has(col.key) ? fixedMap.get(col.key) : col.fixed |
在列设置中新增 fixed 属性的支持