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

Move custom AppearDelay into its own interface #1555

Merged
merged 4 commits into from May 18, 2018

Conversation

4 participants
@peppy
Member

peppy commented May 18, 2018

Avoids the requirement of adding a null AppearDelay to every usage of IHasTooltip

@peppy peppy added the code quality label May 18, 2018

namespace osu.Framework.Graphics.Cursor
{
/// <summary>
/// A tooltip which provides a custom delay until it appears, override the <see cref="TooltipContainer"/>-wide default.

This comment has been minimized.

@Aergwyn

Aergwyn May 18, 2018

Member

is there a typo here? Shouldn't it be overriding?

public interface IHasAppearDelay : IDrawable
{
/// <summary>
/// The delay until the tooltip should be displayed. Null means the <see cref="TooltipContainer.AppearDelay"/> from the <see cref="TooltipContainer"/> containing the <see cref="Drawable"/> implementing this interface will be used.

This comment has been minimized.

@Aergwyn

Aergwyn May 18, 2018

Member

Maybe this doesn't need to mention the null-case anymore? The interface documentation itself mentions this already.

/// <summary>
/// A tooltip which provides a custom delay until it appears, override the <see cref="TooltipContainer"/>-wide default.
/// </summary>
public interface IHasAppearDelay : IDrawable

This comment has been minimized.

@smoogipoo

smoogipoo May 18, 2018

Contributor

Should inherit from IHasTooltip.

This comment has been minimized.

@Tom94

Tom94 May 18, 2018

Collaborator

I disagree, unless the interface is renamed to IHasTooltipWithAppearDelay

This comment has been minimized.

@smoogipoo

smoogipoo May 18, 2018

Contributor

Given the xmldoc, and given the namespacing of the interface, I believe it should inherit.

Would also be fine with naming it IHasTooltipWithAppearDelay.

@smoogipoo smoogipoo merged commit 73e8099 into ppy:master May 18, 2018

2 checks passed

CodeFactor No issues found.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@peppy peppy deleted the peppy:appear-delay-to-custom-interface branch Jul 5, 2018

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