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

feat(ui): Add custom time range option in time range dropdown #11990

Merged
merged 1 commit into from
Feb 20, 2019

Conversation

ischolten
Copy link
Contributor

@ischolten ischolten commented Feb 19, 2019

Closes #11692

Briefly describe your proposed changes:
Add a '
Custom Time Range option to the timerange dropdown that opens a calendar selection to choose the time range
Change the clickOutside component to listen to mousedown rather than click
Add react-datepicker as dependency
add stop to queries built by the query builder that default to now() if a custom time is not selected

  • Rebased/mergeable
  • Tests pass
  • http/swagger.yml updated (if modified Go structs or API)
  • Sign CLA (if not already signed)

@ischolten ischolten force-pushed the feat/time-range-calendar branch 3 times, most recently from 6367b0e to ea26a71 Compare February 19, 2019 21:49
Copy link
Contributor

@chnn chnn left a comment

Choose a reason for hiding this comment

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

Wonderful

private get stylePosition(): CSSProperties {
const {position} = this.props
const {bottomPosition, topPosition} = this.state
const pickerHeight = 410
Copy link
Contributor

Choose a reason for hiding this comment

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

nit Want to extract this to a named screaming snake case constant at the top of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can do

}

const bottomPx =
(bottomPosition || window.innerHeight - top - 15) - pickerHeight / 2
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the 15 for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

some additional padding; i can make into a constant

(bottomPosition || window.innerHeight - top - 15) - pickerHeight / 2
return {
bottom: `${bottomPx}px`,
right: `${right + 2}px`,
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the 2 for?

Copy link
Contributor Author

@ischolten ischolten Feb 20, 2019

Choose a reason for hiding this comment

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

some additional padding; i can make it into a named constant

@ischolten ischolten force-pushed the feat/time-range-calendar branch from ea26a71 to f0e7f30 Compare February 20, 2019 00:31
@ischolten ischolten force-pushed the feat/time-range-calendar branch from f0e7f30 to f9aab0f Compare February 20, 2019 01:12
@ischolten ischolten merged commit d895f5a into master Feb 20, 2019
@ischolten ischolten deleted the feat/time-range-calendar branch February 20, 2019 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants