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 broken referenceKey prop on SelectColumnsButton #8432

Merged
merged 2 commits into from
Dec 8, 2022
Merged

Fix broken referenceKey prop on SelectColumnsButton #8432

merged 2 commits into from
Dec 8, 2022

Conversation

wcoppens
Copy link
Contributor

Fixes #8431

This commit fixes the broken referenceKey prop for the SelectColumnsButton.

Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Test are failing

@wcoppens
Copy link
Contributor Author

@WiXSL My bad, updated the codestyle and added some extra tests to verify working.

Comment on lines 43 to 45
const preferenceKey = props.preferenceKey
? `preferences.${props.preferenceKey}`
: `preferences.${resource}.datagrid`;
Copy link
Member

Choose a reason for hiding this comment

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

For consistency, please use the same syntax as in DatagridConfigurable:

    const finalPreferenceKey = preferenceKey || `${resource}.datagrid`;

    const [availableColumns, setAvailableColumns] = useStore<
        ConfigurableDatagridColumn[]
    >(`preferences.${finalPreferenceKey}.availableColumns`, []);

    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    const [_, setOmit] = useStore<string[]>(
        `preferences.${finalPreferenceKey}.omit`,
        omit
    );

@fzaninotto
Copy link
Member

Sorry, needs rebase

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Could you please also apply the changes regarding the props type that are suggested in #8479

  • typing props with SelectColumnsButtonProps
  • using the sanitizeRestProps function

Also, there are 2 linter warnings you need to fix:
2022-12-08_10-42

Thanks

@antoinefricker
Copy link
Contributor

Hi @wcoppens

I did not see your PR, sorry about that!

@wcoppens
Copy link
Contributor Author

wcoppens commented Dec 8, 2022

Hi @wcoppens

I did not see your PR, sorry about that!

No problem, as long we have the same goal ;-).

I've took your TS typings and Sanitizeprops function and applied a separate commit, thanks!

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Thanks!

@slax57 slax57 added this to the 4.6.1 milestone Dec 8, 2022
@slax57 slax57 merged commit 35ba5ba into marmelab:master Dec 8, 2022
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.

DataGridConfigurable & SelectColumnsButton preferenceKey broken
5 participants