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

Make TooltipContainer and ContextMenuContainer only consider children #873

Merged
merged 23 commits into from
Jul 6, 2017

Conversation

default0
Copy link
Contributor

@default0 default0 commented Jul 5, 2017

This changes the TooltipContainer API to be more reasonable:
Instead of adding the ability for its siblings and parents to have tooltips, the TooltipContainer now enables tooltips for all its children.

The TooltipContainer will now do as its name implies: Enable tooltips
for all its children, instead of for all its siblings in the draw graph.
Changing the sizing of the TooltipContainer could cause exceptions,
nested TooltipContainers were not handled correctly and some drawables
were considered that were not children of the TooltipContainer.
content.AutoSizeAxes = Axes.None;

// in addition to using this.RelativeSizeAxes, sets RelativeSizeAxes on every axis that is neither relative size nor auto size
content.RelativeSizeAxes = RelativeSizeAxes | (Axes.Both & ~(RelativeSizeAxes | AutoSizeAxes));

This comment was marked as off-topic.

@@ -133,6 +157,8 @@ private void hideTooltip()
currentlyDisplayed = null;
}

private readonly HashSet<Drawable> pathDrawables = new HashSet<Drawable>();

This comment was marked as off-topic.

@@ -133,6 +157,8 @@ private void hideTooltip()
currentlyDisplayed = null;
}

private readonly HashSet<Drawable> pathDrawables = new HashSet<Drawable>();
private readonly HashSet<Drawable> nestedPathDrawables = new HashSet<Drawable>();

This comment was marked as off-topic.


tooltipTarget = candidate as IHasTooltip;
if (tooltipTarget != null)
break;

This comment was marked as off-topic.

pathDrawables.Add(candidate);

var nestedTtc = candidate as TooltipContainer;
if (nestedTtc != null || nestedPathDrawables.Contains((Drawable)candidate.Parent))

This comment was marked as off-topic.

IHasTooltip tooltipTarget = null;
foreach (var candidate in targetCandidates)
{
if (!pathDrawables.Contains((Drawable)candidate.Parent))

This comment was marked as off-topic.

if (nestedTtc != null || nestedPathDrawables.Contains((Drawable)candidate.Parent))
{
nestedPathDrawables.Add(nestedTtc ?? candidate);
continue;

This comment was marked as off-topic.

var nestedTtc = candidate as TooltipContainer;
if (nestedTtc != null || nestedPathDrawables.Contains((Drawable)candidate.Parent))
{
nestedPathDrawables.Add(nestedTtc ?? candidate);

This comment was marked as off-topic.

Copy link
Collaborator

@Tom94 Tom94 left a comment

Choose a reason for hiding this comment

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

Gonna get this in since this has been heavily reviewed by both @default0 and me offline.

@Tom94 Tom94 changed the title TooltipContainer API Change Make TooltipContainer and ContextMenuContainer only consider children Jul 6, 2017
@Tom94 Tom94 merged commit 862e478 into ppy:master Jul 6, 2017
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