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

feat: add filterTooltipValueFn option to transform the value of the filter tooltip #360

Merged

Conversation

yamadayutaka
Copy link
Contributor

This PR is an implementation of the feature suggestion #359.

Specify the Column Option as follows.

{
  ...
  filterTooltipValueFn: (value: any) => {
    const d = dayjs(value || '');
    return d.isValid() ? d.format('L') : '';
  },
  ...
},

Then, the tooltip is displayed as follows.
image

Copy link

vercel bot commented Jun 14, 2024

@yamadayutaka is attempting to deploy a commit to the Kevin Vandy OSS Team on Vercel.

A member of the Team first needs to authorize it.

@alessandrojcm
Copy link
Collaborator

Hi @yamadayutaka thanks for your contribution, please see the comments I left.

@yamadayutaka
Copy link
Contributor Author

Hi @alessandrojcm thanks for your reply!
But I can't check your comments.
I think you may have forgotten to "Submit review".
Could you confirm this?

@@ -69,10 +71,12 @@ export const MRT_TableHeadCellFilterLabel = <TData extends MRT_RowData>({
'{filterValue}',
`"${
Array.isArray(column.getFilterValue())
? (column.getFilterValue() as [string, string]).join(
? (column.getFilterValue() as [])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure here about the type, if the previous type was casted as [string, string] then it should remain so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since filterTooltipValueFn does the conversion to string, I think the input to filterTooltipValueFn should remain the original data type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok yeah agree, do you think you could type the prop accordingly then? Basically, I´d like to avoid using any when possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a type definition. What do you think of this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @yamadayutaka could please add it to the prop? So it'd be filterToolValueFn?: (value: string) => string

@@ -500,6 +500,7 @@ export type MRT_ColumnDef<TData extends MRT_RowData, TValue = unknown> = Omit<
enableEditing?: ((row: MRT_Row<TData>) => boolean) | boolean;
enableFilterMatchHighlighting?: boolean;
filterFn?: MRT_FilterFn<TData>;
filterTooltipValueFn?: (value: any) => string,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

@alessandrojcm
Copy link
Collaborator

Hi @alessandrojcm thanks for your reply! But I can't check your comments. I think you may have forgotten to "Submit review". Could you confirm this?

Yes apologies, please check now.

Copy link
Collaborator

@alessandrojcm alessandrojcm left a comment

Choose a reason for hiding this comment

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

Sorry missed that you already typed the props (GH's UI can be very confusing sometimes...) Wil merge now thanks!

@alessandrojcm alessandrojcm merged commit de06f8a into KevinVandy:v2 Jul 17, 2024
0 of 3 checks passed
@yamadayutaka
Copy link
Contributor Author

Thanks for merging!

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.

3 participants