-
Notifications
You must be signed in to change notification settings - Fork 29
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
Make timestamp column hideable and draggable #2239
Changes from 13 commits
3f775a2
64f0b6a
eda9ff3
2f69114
03c2cea
caba7c7
49ffe3c
2294818
b10574a
aa98a23
0b1470e
4c8c433
d77d768
e90d2d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import { ColumnType } from '../webview/contract' | ||
|
||
export const timestampColumn = { | ||
hasChildren: false, | ||
label: 'Timestamp', | ||
parentPath: ColumnType.TIMESTAMP, | ||
path: 'Timestamp', | ||
type: ColumnType.TIMESTAMP | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import { ColumnLike } from './like' | ||
import { timestampColumn } from './constants' | ||
import { definedAndNonEmpty } from '../../util/array' | ||
import { | ||
QuickPickOptionsWithTitle, | ||
|
@@ -14,11 +15,13 @@ export const pickFromColumnLikes = ( | |
return Toast.showError('There are no columns to select from.') | ||
} | ||
return quickPickValue<ColumnLike>( | ||
columnLikes.map(columnLike => ({ | ||
description: columnLike.path, | ||
label: columnLike.label, | ||
value: columnLike | ||
})), | ||
columnLikes | ||
.filter(columnLike => columnLike.path !== timestampColumn.path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [C] I do not think we want to filter this here. We will actually want to make it possible to sort/filter by timestamp in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is definitely causing at least one test failure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I did test out sorting with timestamp but it wasn't working so I disabled both for now :) |
||
.map(columnLike => ({ | ||
description: columnLike.path, | ||
label: columnLike.label, | ||
value: columnLike | ||
})), | ||
quickPickOptions | ||
) | ||
} |
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.
Is there a reason for using the capitalized
Timestamp
?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.
Using a capitalized Timestamp helps us to write less code since that's what the label is referred to as :)
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 now would be a good time to rename this to
Created
to match the CLI table:Can do it in follow up if necessary.