-
Notifications
You must be signed in to change notification settings - Fork 379
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
Refactor user profile page initial data #9324
Refactor user profile page initial data #9324
Conversation
…e with other sections
...probably for years
7f2d906
to
f1abea3
Compare
@@ -118,6 +141,86 @@ public function checkUsernameExists() | |||
return json_item($user, 'UserCompact', ['cover', 'country']); | |||
} | |||
|
|||
public function extraPages($_id, $page) | |||
{ | |||
if (!in_array($page, static::LAZY_EXTRA_PAGES, true)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be just default case
$initialData = [ | ||
'achievements' => $achievements, | ||
'current_mode' => $currentMode, | ||
'extras' => $extras, | ||
'per_page' => $perPage, | ||
'per_page' => static::PER_PAGE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this can be moved as part of extra page data
return [ | ||
'favourite' => [ | ||
'count' => $this->user->profileBeatmapsetsFavourite()->count(), | ||
'items' => $this->getExtra($this->user, 'favouriteBeatmapsets', [], static::PER_PAGE['favouriteBeatmapsets']), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like can be made into function (the +1
limit can be $extraLimit
parameter or something) or maybe pass count (and do the limit thing if no count is passed for has_more
data).
also not sure why $this->user
and $this->mode
are explicitly passed when everything calling this function passes the same thing.
}; | ||
} | ||
|
||
for (const key of Object.keys(this.state.scores) as (keyof TopScoresJson)[]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also for beatmapsets above (or maybe not needed if the server returns correct json in the first place)
topScoresJsonKeys = ['best', 'firsts', 'pinned'] as const;
type TopScoresJson = Record<(typeof topScoresJsonKeys)[number], ExtraPageJson<SoloScoreJsonForUser>>;
// use topScoresJsonKeys for this loop
scoresRecent: SoloScoreJsonForUser[]; | ||
} | ||
|
||
export interface ExtraPageJson<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more like PaginatedPageSectionJson
. And maybe extend the WithoutCount
version instead
export interface ExtraPageJson<T> { | ||
count: number; | ||
items: T[]; | ||
pagination: OffsetPaginationJson; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a lie as the original json doesn't contain this
maybe update the controller to actually return the correct pagination data instead. It would be even better if the item is discarded before turned into json.
historical: HistoricalJson; | ||
kudosu: ExtraPageJsonWithoutCount<KudosuHistoryJson>; | ||
recentActivity: ExtraPageJsonWithoutCount<EventJson>; | ||
scores: TopScoresJson; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
topScores
?
const beatmapsets = this.props.controller.state.extras[section.key]; | ||
const pagination = this.props.controller.state.pagination[section.key]; | ||
const state = this.props.controller.state.beatmapsets; | ||
const count = state[section.translationKey].count; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
translationKey
should just be called key
and key
should be urlKey
or something along that line.
since show() sets $this->user via parsePaginationParams now
move b682fe5ed4 to this branch
making pagination optional instead of having the extra types had its own set of typing problems...
...one of the reasons we did ppy#9324 in the first place
Part 1 of ?
Refactoring the initial data for the user profile page to better support future lazy loading of extra page tab sections.
The initial data structure to make it more convenient for the lazy loaded components to handle.
LazyLoad
component will be a separate PR (basically a wrapper withIntersectionObserver
)