-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 advanced setting to control quick ranges #15975
Changes from all commits
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 |
---|---|---|
@@ -1,20 +1,20 @@ | ||
import _ from 'lodash'; | ||
import dateMath from '@elastic/datemath'; | ||
import moment from 'moment'; | ||
import 'ui/timepicker/quick_ranges'; | ||
import 'ui/timepicker/time_units'; | ||
import { uiModules } from 'ui/modules'; | ||
const module = uiModules.get('kibana'); | ||
|
||
|
||
module.directive('prettyDuration', function (config, quickRanges, timeUnits) { | ||
module.directive('prettyDuration', function (config, timeUnits) { | ||
return { | ||
restrict: 'E', | ||
scope: { | ||
from: '=', | ||
to: '=' | ||
}, | ||
link: function ($scope, $elem) { | ||
const quickRanges = config.get('timepicker:quickRanges'); | ||
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. Do we need a check here to make sure users don't add a crazy number of sections? 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. Meh, we could, but I don't envision that being a super common problem and it's easily fixed. |
||
const dateFormat = config.get('dateFormat'); | ||
|
||
const lookupByRange = {}; | ||
|
This file was deleted.
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 way to add a
verify
method to ensure the time ranges are valid? It would be better to get an error message and not update the quickRanges on the advanced setting page, then to let users save invalid time ranges and get a cryptic error when selecting the invalid range via the timepicker.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 don't think there's any sort of verification currently (beyond verifying that the value actually is JSON). I could probably add a custom directive and handling for this but it might add more complexity than it's worth... Plus, there are other JSON values in the advanced settings that we don't have any sort of verification either. What if I just added some notes to the docs about how to configure the value and linked to it in the description?
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.
sounds good
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.
@nreese 1eb0eee