-
-
Notifications
You must be signed in to change notification settings - Fork 527
feat: Update to pin actions in show/hide columns menu #1423
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: v3
Are you sure you want to change the base?
Conversation
jrassa
commented
Jun 12, 2025
- Add option to enable/disable 'Unpin all'. Enabled by default.
- Add new 'Reset pins' action that restores pinning to initial state.
- Add option to enable/disable 'Reset pins' action. Disabled by default.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
f75fd56 to
2d26461
Compare
2d26461 to
71745f0
Compare
* Add option to enable/disable 'Unpin all'. Enabled by default. * Add new 'Reset pins' action that restores pinning to initial state. * Add option to enable/disable 'Reset pins' action. Disabled by default.
71745f0 to
443bf1b
Compare
| {localization.resetPins} | ||
| </Button> | ||
| )} | ||
| {enableHiding && ( |
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.
Disclaimer, I have no merging power but hopefully this gets the discussion going.
Recently, I had to solve for this myself in a custom override.
renderToolbarInternalActions - there was no renderShowHideColumnMenuHeader tie in so I used renderToolbarInternalActions and copied a lot of the existing functionality and changed what I needed to move forward.
Here are some notes on this pr:
- The code looks correct.
- The vercel deployment is failing not sure if that is why the pr is sitting. I took a look but was unable to see the deployment logs they might not be public.
- Does the additional item in the show hide menu row make it look cramped? Adding a screenshot here could help I might add one of your version in a bit.
- This MRT_ShowHideColumns menu does multiple actions therefore does it make sense to have a general reset to default states for all actions? In my implementation I have commented out all of the other sections and just have the following generic reset:
{enableColumnPinning && (
<Button
disabled={!getIsSomeColumnsPinned()}
onClick={() => {
table.setColumnVisibility(table.initialState.columnVisibility);
table.resetColumnPinning();
table.setColumnOrder(getDefaultColumnOrderIds(table.options, true)
}}
style={{ marginRight: '1.75rem', float: right }}
>
Reset
</Button>
)}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.
update: @jrassa, @KevinVandy. the build is failing because all of the locale's have not been updated:
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 "Column Pinning Initial" storybook was updated to enable this option so it is good place to see how it looks. It doesn't look cramped, but the menu is wider with the extra option.
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.
cool, I did not think to look at the storybook preview. thanks
MichaelDimmitt
left a comment
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.
build failing because not all locales have been updated.
Not sure the proper way to fix this. I could fill in each with the english value, but that feels wrong as it already falls back to the EN local. The proper solution would probably be to update the types in the locales definitions. I see two possible approaches:
I can update to do either approach. Any insight @KevinVandy? |
|
I was thinking a third option to what you suggested could be do a google translate on the key in various languages. find a way to make adding that as easy as possible possibly with a command line tool. @jrassa |
|
@jrassa @KevinVandy , I pointed a pr on top of this one that did a best effort at adding those locales to fix the build. This just gives that option if you guys want the build fixed. |
|
A last option would be to add a new tie in: |
