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 recent section to user profile #2120

Merged
merged 17 commits into from Mar 7, 2018

Conversation

2 participants
@naoey
Contributor

naoey commented Feb 25, 2018

Depends on:

naoey added some commits Feb 25, 2018

Create drawable and add response to profile.
- Add missing JSON fields to response model
- Add missing enum value
Add link handling to recent activities.
- Add a show user action to link handling

@peppy peppy added this to the February 2018 milestone Feb 26, 2018

naoey added some commits Feb 26, 2018

}
}
private string urlToAbsolute(string url) => $"{api?.Endpoint ?? @"https://osu.ppy.sh"}{url}";

This comment has been minimized.

@naoey

naoey Feb 26, 2018

Contributor

this seems unnecessary, but i'm unsure whether taking endpoint from api or hardcoding is preferred. or whether to use osu:// links.

This comment has been minimized.

@peppy

peppy Feb 27, 2018

Member

using the api is fine, but there's no case the api is null, so the fallback is not required.

naoey and others added some commits Feb 27, 2018

Fix long recent activity text overlapping timestamp.
- Also remove unnecessary fallback from absolute URL helper
Remove redundant subsection title.
- Also handle opening UserProfile in LinkFlowContainer similar to how
beatmaps and channels are handled

@peppy peppy modified the milestones: February 2018, March 2018 Mar 1, 2018

{
this.api = api;
userLinkTemplate = $"[{toAbsoluteUrl(activity.User?.Url)} {activity.User?.Username}]";

This comment has been minimized.

@peppy

peppy Mar 3, 2018

Member

i'd probably move these inside activityToString() as local functions rather than assigning them all in BDL (doesn't seem to be any reason for doing it this early).

i'd also consider moving the MessageFormatter.FormatText call in that method and rename it to createMessage() instead of the awkward activityToString().

private MessageFormatter.MessageFormatterResult createMessage()
{
string userLinkTemplate = $"[{toAbsoluteUrl(activity.User?.Url)} {activity.User?.Username}]";

This comment has been minimized.

@peppy

peppy Mar 5, 2018

Member

was hoping for local methods
ie.

string userLinkTemplate() => $"...";

This means it won't unnecessarily string interpolate the pieces which aren't required.

naoey and others added some commits Mar 3, 2018

@peppy

This comment has been minimized.

Member

peppy commented Mar 7, 2018

Looking good, apart from missing VisualTest!

naoey and others added some commits Mar 7, 2018

@peppy

peppy approved these changes Mar 7, 2018

@peppy peppy merged commit ca249ab into ppy:master Mar 7, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@naoey naoey deleted the naoey:user-profile-recent branch Mar 10, 2018

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