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

[Discover] account for hidden time column in default sort #129659

Conversation

drewdaemon
Copy link
Contributor

Summary

Fixes #121263

The time column is no longer assigned as the default sort when it is hidden via advanced settings.

Screen Shot 2022-04-06 at 3 28 07 PM

Checklist

@drewdaemon drewdaemon changed the title [Lens] account for hidden time column in default sort [Discover] account for hidden time column in default sort Apr 6, 2022
@drewdaemon drewdaemon added Feature:Discover Discover Application release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.2.0 v8.3.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Apr 6, 2022
@drewdaemon drewdaemon marked this pull request as ready for review April 6, 2022 20:42
@drewdaemon drewdaemon requested a review from a team as a code owner April 6, 2022 20:42
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

Comment on lines +216 to +221
private getDefaultSort(dataView?: DataView) {
const defaultSortOrder = this.services.uiSettings.get(SORT_DEFAULT_ORDER_SETTING, 'desc');
const hidingTimeColumn = this.services.uiSettings.get(DOC_HIDE_TIME_COLUMN_SETTING, false);
return getDefaultSort(dataView, defaultSortOrder, hidingTimeColumn);
}

Copy link
Member

Choose a reason for hiding this comment

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

nice cleanup ❤️

Comment on lines +216 to +221
private getDefaultSort(dataView?: DataView) {
const defaultSortOrder = this.services.uiSettings.get(SORT_DEFAULT_ORDER_SETTING, 'desc');
const hidingTimeColumn = this.services.uiSettings.get(DOC_HIDE_TIME_COLUMN_SETTING, false);
return getDefaultSort(dataView, defaultSortOrder, hidingTimeColumn);
}

Copy link
Member

Choose a reason for hiding this comment

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

nice cleanup ❤️

Copy link
Member

Choose a reason for hiding this comment

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

well, seems I clicked to often, or GitHub has issues?

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, this fixes the linked issue, thx a lot 👍 Tested with Firefox, Chrome, Safari, works as expected.

One think worth a follow up. When you have an existing saved search with sorting by time field in the saved object, you can't unsort when not using the Document explorer.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default CI Group #10 / Actions and Triggers app Rule Details Header should reenable a disabled the rule

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
discover 400.4KB 400.4KB +71.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.2

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Discover Discover Application release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.2.0 v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sorting in Discover not working with "Hide Time Column" setting
5 participants