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

Add badges to ProfileHeader #2407

Merged
merged 8 commits into from Apr 18, 2018

Conversation

3 participants
@jorolf
Contributor

jorolf commented Apr 15, 2018

No description provided.

jorolf added some commits Apr 15, 2018

@peppy peppy added this to the April 2018 milestone Apr 16, 2018

private void onOuterHoverLost()
{
rotateBadges();

This comment has been minimized.

@smoogipoo

smoogipoo Apr 16, 2018

Contributor

With this here, the badge index gets incremented every time you mouse in then out of the container. Web only scrolls into the previously-displayed icon on mouseout.

private int visibleBadge;
private int badgeCount;
public void ShowBadges(Badge[] badges)

This comment has been minimized.

@smoogipoo

smoogipoo Apr 16, 2018

Contributor

Is there a reason to not pass in the User via ctor, and then load their badges in bdl load()?

This comment has been minimized.

@jorolf

jorolf Apr 16, 2018

Contributor

The BadgeContainer is created in the ctor of ProfileHeader which does not have a "complete" User

{
private const float outer_container_width = 98;
private const float outer_container_padding = 3;
private static readonly Vector2 badge_size = new Vector2(outer_container_width - outer_container_padding * 2, 46);

This comment has been minimized.

@smoogipoo

smoogipoo Apr 16, 2018

Contributor

I'd really like this to be reversed. badge_size should be 86x40, and everything else should be padded around that to achieve the desired sizes.

As it currently is, I had to go into draw vis + check childsize to make sure that the badges were the correct size.

},
Badges = badges
};
}

This comment has been minimized.

@smoogipoo

smoogipoo Apr 16, 2018

Contributor

Instead of repurposing this testcase for badges, create another testcase with just the BadgeContainer in it.

We want to test individual components in general.

Texture = textures.Get(badge.ImageUrl),
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
OnLoadComplete = d => d.FadeInFromZero(200)

This comment has been minimized.

@peppy

peppy Apr 18, 2018

Member

this is pointless because you're already in a blocking context here. just over to LoadComplete of DrawableBadge instead.

all fixed

@peppy

peppy approved these changes Apr 18, 2018

@peppy peppy merged commit f678ce1 into ppy:master Apr 18, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment