Add header filtering to query result grid #18373
Conversation
cssuh
requested review from
kburtram,
aasimkhan30,
caohai,
laurennat,
lewis-sanchez and
Benjin
as code owners
PR Changes
|
|
Can you update the PR title to be more specific? Probably something like |
| @@ -238,6 +376,86 @@ export class HeaderFilter<T extends Slick.SlickData> { | |||
| this.setFocusToColumn(columnDef); | |||
| } | |||
|
|
|||
| private async createFilterList(): Promise<void> { | |||
There was a problem hiding this comment.
Does this list need to be virtualized?
There was a problem hiding this comment.
@cssuh Can you test filtering with something like 10k different rows?
| apply: l10n.t("Apply"), | ||
| clear: l10n.t("Clear"), | ||
| search: l10n.t("Search..."), | ||
| selected: l10n.t("0 Selected"), |
There was a problem hiding this comment.
When is this shown? My gut says that this either ought to be "{num} selected" or "None selected" (word rather than number for the special "zero-case" feels nicer here to me?)
| apply: l10n.t("Apply"), | ||
| clear: l10n.t("Clear"), | ||
| search: l10n.t("Search..."), | ||
| selected: l10n.t("0 Selected"), |
There was a problem hiding this comment.
If this is only shown when none are selected, then the var name should be noneSelected or zeroSelected instead of selected
|
Can you add a video to the PR description so we can see it in action? |
| if ( | ||
| this.theme === ColorThemeKind.Dark || | ||
| this.theme === ColorThemeKind.HighContrast | ||
| ) { | ||
| $el.addClass("vs-dark"); | ||
| } else { | ||
| $el.addClass("vs-light"); | ||
| } |
There was a problem hiding this comment.
Do you know how "vs-light" differs from just "light"? Could this just use utils.resolveVscodeThemeType(themeKind) instead?
There was a problem hiding this comment.
Quick question, why are we adding classes to the component?
Is using vscode css variables not enough in this case? We should generally just stick to them for all of our coloring and styling needs.
Currently, this doesn't support vs Hight Contrast light theme.
Addresses: #18063
Addresses: #18250
Adds filtering to the header menu
TODO: