Skip to content

Conversation

@edwardgou-sentry
Copy link
Contributor

Adds support for World Map display type in Discover.
World Map can be selected in the Display dropdown.
World Map only supports single Y-Axis selections. Any selection of > 1 Y-Axis will default to Total Period display.

image
image

@edwardgou-sentry edwardgou-sentry requested review from a team October 18, 2021 20:25
@edwardgou-sentry edwardgou-sentry requested a review from a team as a code owner October 18, 2021 20:25
@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2021

size-limit report

Path Base Size (efbb678) Current Size Change
src/sentry/static/sentry/dist/entrypoints/app.js 52.74 KB 52.77 KB +0.07% 🔺
src/sentry/static/sentry/dist/entrypoints/sentry.css 70.89 KB 70.89 KB 0%

Copy link
Member

@wmak wmak left a comment

Choose a reason for hiding this comment

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

LGTM but two questions:

World Map only supports single Y-Axis selections. Any selection of > 1 Y-Axis will default to Total Period display.

Should we just disable the ability to select >1 y-axis in world map?

And will there be a follow up PR to update open in discover on world maps with display mode?

@edwardgou-sentry
Copy link
Contributor Author

edwardgou-sentry commented Oct 20, 2021

LGTM but two questions:

World Map only supports single Y-Axis selections. Any selection of > 1 Y-Axis will default to Total Period display.

Should we just disable the ability to select >1 y-axis in world map?

And will there be a follow up PR to update open in discover on world maps with display mode?

Hey @wmak , we can definitely disable selecting >1 y-axis for World Map view. For Top Period and Top Daily views, they also just default back to Total Period and Total Daily at the moment. Should we update these as well to disable selecting >1 y-axis? I'm thinking all of these can be handled in a new PR.

For open in discover, yup we will create a new PR to open in World Map display mode!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants