Skip to content

fix(discover): Disable top 5 mode when no aggregates present#19796

Merged
Zylphrex merged 2 commits into
masterfrom
fix/disable-top-5-mode-when-no-aggregates
Jul 10, 2020
Merged

fix(discover): Disable top 5 mode when no aggregates present#19796
Zylphrex merged 2 commits into
masterfrom
fix/disable-top-5-mode-when-no-aggregates

Conversation

@Zylphrex

@Zylphrex Zylphrex commented Jul 9, 2020

Copy link
Copy Markdown
Member

Top 5 mode should only be available when there is at least one aggregate. If
there are no aggregate present, disable top 5 modes and default back to total
period. This will also cause the top 5 indicators to be hidden when top 5 mode
is turned off.

Top 5 mode should only be available when there is at least one aggregate. If
there are no aggregate present, disable top 5 modes and default back to total
period. This will also cause the top 5 indicators to be hidden when top 5 mode
is turned off.

@markstory markstory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, should we also remove top5 modes when doing a drilldown navigation? Right now if you open a stack when top5 is active the chart stays in top5 and only shows a few spikes. Converting the top5 -> and top5daily -> daily would give better results.

@Zylphrex

Copy link
Copy Markdown
Member Author

Looks good, should we also remove top5 modes when doing a drilldown navigation? Right now if you open a stack when top5 is active the chart stays in top5 and only shows a few spikes. Converting the top5 -> and top5daily -> daily would give better results.

When you click open stack, the aggregates are removed. So what happens is that the graph switches to total period mode, this behaviour makes sense for top 5 period. I do see what you mean about top 5 daily. Switching it to total daily would make more sense than defaulting back to to total period. I'll update that.

@markstory markstory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👏👏

@Zylphrex Zylphrex merged commit e5ec2cf into master Jul 10, 2020
@Zylphrex Zylphrex deleted the fix/disable-top-5-mode-when-no-aggregates branch July 10, 2020 19:19
@benvinegar

Copy link
Copy Markdown
Contributor

Reminder: preference to disable/grey out vs. hide.

@github-actions github-actions Bot locked and limited conversation to collaborators Dec 18, 2020
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.

3 participants