Fix home page not using site-level listing type #1612 #1778
Conversation
jcgurango
requested review from
dessalines,
SleeplessOne1917,
alectrocute and
jsit
as code owners
| if (isBrowser()) { | ||
| const searchParams = new URLSearchParams(window.location.search); | ||
|
|
||
| return Array.from(Object.entries(processors)).reduce( | ||
| (acc, [key, process]) => ({ | ||
| ...acc, | ||
| [key]: process(searchParams.get(key)), | ||
| [key]: process(searchParams.get(key), site), |
There was a problem hiding this comment.
Why would a GetSiteResponse necessarily have properties that should be query params?
There was a problem hiding this comment.
It's for implementing the fallback for listing type to the site-level setting set in the site admin. If later sort type becomes settable in the admin it'd go for that too. Would it make more sense to do something like return null in these callbacks and do the fallback on the home component?
There was a problem hiding this comment.
Would it make more sense to do something like return null in these callbacks and do the fallback on the home component?
Yes. You don't want any of these props in the query params since no page uses them:
export interface GetSiteResponse {
site_view: SiteView;
admins: Array<PersonView>;
version: string;
my_user?: MyUserInfo;
all_languages: Array<Language>;
discussion_languages: Array<LanguageId>;
taglines: Array<Tagline>;
custom_emojis: Array<CustomEmojiView>;
}There was a problem hiding this comment.
Ok I've updated that.
This is a fix for #1612