-
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
Add option to hide a column from the experiments table #1756
Conversation
@@ -407,6 +407,8 @@ export class Experiments extends BaseRepository<TableData> { | |||
) | |||
case MessageFromWebviewType.TOGGLE_EXPERIMENT: | |||
return this.setExperimentStatus(message.payload) | |||
case MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN: |
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.
[Q] Does this fire a telemetry event? Can we add one if it doesn't?
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.
Thanks for the reminder!
I don't know if it does, but I think it's a good idea to have separate tm calls for webview requests, wdyt?
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 should be a different event per message from webview type. For example:
private setColumnOrder(order: string[]) {
this.columns.setColumnOrder(order)
sendTelemetryEvent(
EventName.VIEWS_EXPERIMENTS_TABLE_COLUMNS_REORDERED,
undefined,
undefined
)
}
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.
Sometimes a separate event won't be possible though. E.g if we are calling an internal/registered command that already has one.
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.
👍
setColumnSort(order) | ||
}} | ||
/> | ||
<VSCodeDivider></VSCodeDivider> |
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.
A little nit-picky, but can this be self closing?
8024d9a
to
d3933d1
Compare
bfc4071
to
6b6a328
Compare
Code Climate has analyzed commit 6b6a328 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.8% (0.0% change). View more on Code Climate. |
Closes #1709
This PR adds an option to the Experiments Table Header context menu to hide that specific column.
Screen.Recording.2022-05-23.at.14.40.25.mov