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

Remove filter props #2412

Merged
merged 15 commits into from
May 19, 2021
Merged

Remove filter props #2412

merged 15 commits into from
May 19, 2021

Conversation

amanmahajan7
Copy link
Contributor

@amanmahajan7 amanmahajan7 commented May 18, 2021

Before
image

After
image

src/DataGrid.tsx Outdated Show resolved Hide resolved
import faker from 'faker';
import { css } from '@linaria/core';

import DataGrid from '../../src';
import type { Column, Filters } from '../../src';
import { NumericFilter } from './components/Filters';
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 think NumericFilter is complex and not needed for examples.

@@ -1,11 +1,10 @@
import { useMemo, useState } from 'react';
import Select from 'react-select';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed react-select filter and using datalist instead to keep things simple

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we use react-select at all anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used in one of the editors. We can remove it in the next PR

onChange={(e) =>
setFilters({
...filters,
complete: Number.isFinite(e.target.valueAsNumber)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL e.target.valueAsNumber

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL

@amanmahajan7 amanmahajan7 marked this pull request as ready for review May 19, 2021 13:01
src/DataGrid.tsx Outdated Show resolved Hide resolved
src/DataGrid.tsx Outdated Show resolved Hide resolved
src/DataGrid.tsx Outdated Show resolved Hide resolved
src/style/header.ts Outdated Show resolved Hide resolved
@@ -1,11 +1,10 @@
import { useMemo, useState } from 'react';
import Select from 'react-select';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we use react-select at all anymore?

stories/demos/HeaderFilters.tsx Outdated Show resolved Hide resolved
stories/demos/HeaderFilters.tsx Outdated Show resolved Hide resolved
stories/demos/HeaderFilters.tsx Outdated Show resolved Hide resolved
onChange={(e) =>
setFilters({
...filters,
complete: Number.isFinite(e.target.valueAsNumber)
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL

stories/demos/HeaderFilters.tsx Show resolved Hide resolved
amanmahajan7 and others added 7 commits May 19, 2021 08:36
Co-authored-by: Nicolas Stepien <567105+nstepien@users.noreply.github.com>
Co-authored-by: Nicolas Stepien <567105+nstepien@users.noreply.github.com>
Co-authored-by: Nicolas Stepien <567105+nstepien@users.noreply.github.com>
Co-authored-by: Nicolas Stepien <567105+nstepien@users.noreply.github.com>
Co-authored-by: Nicolas Stepien <567105+nstepien@users.noreply.github.com>
stories/demos/HeaderFilters.tsx Outdated Show resolved Hide resolved
stories/demos/HeaderFilters.tsx Show resolved Hide resolved
stories/demos/HeaderFilters.tsx Outdated Show resolved Hide resolved
stories/demos/HeaderFilters.tsx Outdated Show resolved Hide resolved
stories/demos/HeaderFilters.tsx Outdated Show resolved Hide resolved
Co-authored-by: Nicolas Stepien <567105+nstepien@users.noreply.github.com>
Copy link
Contributor

@nstepien nstepien left a comment

Choose a reason for hiding this comment

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

:shipit:

@amanmahajan7 amanmahajan7 merged commit 76c5bb9 into canary May 19, 2021
@amanmahajan7 amanmahajan7 deleted the am-remove-filters branch May 19, 2021 14:36
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.

None yet

2 participants