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 positioning option to line graphs #16077

Merged
merged 2 commits into from Dec 4, 2019

Conversation

@TCL735
Copy link
Contributor

TCL735 commented Dec 2, 2019

Closes #15836
Closes #15808
Add dropdown menu for line positioning in graphs to allow for stacked line graphs.

@TCL735 TCL735 requested review from influxdata/api as code owners Dec 2, 2019
@TCL735 TCL735 force-pushed the feat_15836_add_stacked_line_layer branch from fd10792 to d399119 Dec 2, 2019
Copy link
Contributor

hoorayimhelping left a comment

easy peasy lemon squeezy

http/swagger.yml Show resolved Hide resolved
@Asalem1
Asalem1 approved these changes Dec 3, 2019
Copy link
Contributor

Asalem1 left a comment

Looks great! Way to go. Does giraffe handle instances where position is undefined?

@TCL735

This comment has been minimized.

Copy link
Contributor Author

TCL735 commented Dec 3, 2019

There is an issue that needs to resolved: user selection of line position does not persist when cell is saved into a dashboard, and then the cell is viewed in the dashboard. Line position selection is empty and graph defaults to "overlaid" even when "stacked" was selected prior to saving.

@TCL735

This comment has been minimized.

Copy link
Contributor Author

TCL735 commented Dec 3, 2019

Looks great! Way to go. Does giraffe handle instances where position is undefined?

Yes, not giving a position (undefined) is handled through TypeScript. We have an interface that says position is optional. At compile time, TypeScript will convert to JavaScript with the necessary properties (with undefined values for optional properties)

The behavior of line graphs without a position property is "overlaid"

@TCL735 TCL735 force-pushed the feat_15836_add_stacked_line_layer branch 2 times, most recently from abee34f to 3d2e2bb Dec 3, 2019
@kelwang kelwang requested a review from jsteenb2 Dec 3, 2019
@TCL735 TCL735 force-pushed the feat_15836_add_stacked_line_layer branch from 3d2e2bb to a0a8b83 Dec 3, 2019
@TCL735 TCL735 force-pushed the feat_15836_add_stacked_line_layer branch from a0a8b83 to 596a7a0 Dec 3, 2019
dashboard.go Show resolved Hide resolved
…line plus stat charts
@TCL735 TCL735 merged commit 5150039 into master Dec 4, 2019
7 of 8 checks passed
7 of 8 checks passed
continuous-integration/jenkins/branch This commit is being built
Details
Semantic Pull Request ready to be merged, squashed or rebased
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: e2e Your tests passed on CircleCI!
Details
ci/circleci: gotest Your tests passed on CircleCI!
Details
ci/circleci: jstest Your tests passed on CircleCI!
Details
ci/circleci: litmus_daily Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@jsteenb2 jsteenb2 mentioned this pull request Dec 4, 2019
3 of 3 tasks complete
@TCL735 TCL735 deleted the feat_15836_add_stacked_line_layer branch Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.