-
Notifications
You must be signed in to change notification settings - Fork 31
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: disable column sorting on unsupported types #1390
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1390 +/- ##
==========================================
- Coverage 45.59% 45.57% -0.02%
==========================================
Files 506 506
Lines 34975 34993 +18
Branches 8737 8749 +12
==========================================
+ Hits 15947 15949 +2
- Misses 18977 18993 +16
Partials 51 51
Flags with carried forward coverage won't be shown. Click here to find out more.
|
packages/iris-grid/src/mousehandlers/IrisGridContextMenuHandler.tsx
Outdated
Show resolved
Hide resolved
@@ -54,7 +54,7 @@ function makeAdvancedFilterCreatorWrapper( | |||
selectedValues: [], | |||
}, | |||
model: irisGridTestUtils.makeModel(), | |||
column: irisGridTestUtils.makeColumn(), | |||
column: irisGridTestUtils.makeColumn('0'), // needs to match column name in makeModel so that columnIndex can be found when rendering |
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.
There's probably a more fool-proof way to achieve this?
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.
Maybe I should be handling that null/undefined case differently in AdvancedFilterCreator.tsx
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.
I think this is fine. Anything else you'd do would be a bit long-winded and unnecessary. You could define the columns beforehand, for example, so something like:
const columns = irisGridTestUtils.makeColumns(3, 'test');
...
function makeAdvancedFilterCreatorWrapper(
....
} = {
model: irisGridTestUtils.makeModel(irisGridTestUtils.makeTable(columns)),
column: column[0],
There's still a lot of tests here disabled, we should really look at re-enabling them.
@@ -54,7 +54,7 @@ function makeAdvancedFilterCreatorWrapper( | |||
selectedValues: [], | |||
}, | |||
model: irisGridTestUtils.makeModel(), | |||
column: irisGridTestUtils.makeColumn(), | |||
column: irisGridTestUtils.makeColumn('0'), // needs to match column name in makeModel so that columnIndex can be found when rendering |
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.
I think this is fine. Anything else you'd do would be a bit long-winded and unnecessary. You could define the columns beforehand, for example, so something like:
const columns = irisGridTestUtils.makeColumns(3, 'test');
...
function makeAdvancedFilterCreatorWrapper(
....
} = {
model: irisGridTestUtils.makeModel(irisGridTestUtils.makeTable(columns)),
column: column[0],
There's still a lot of tests here disabled, we should really look at re-enabling them.
Closes #1380