Follower sorting #6184
Follower sorting #6184
Conversation
hipstersmoothie
changed the title
hipstersmoothie
changed the title
(cherry picked from commit 96a5969111329dfc333224162505acbf302d8c19)
(cherry picked from commit bd8f8f1090564d1ce0c6de848ccc6e46c5c86adc)
(cherry picked from commit 1ffdff0590d2a77a189357633ab8d0edb683868f)
| @@ -96,3 +98,19 @@ export function isPlainObject(o: any): o is Object { | |||
| function hasObjectPrototype(o: any): boolean { | |||
| return Object.prototype.toString.call(o) === '[object Object]' | |||
| } | |||
|
|
|||
| export function sortProfiles( | |||
There was a problem hiding this comment.
Wasn't sure where to put this function.
|
are you sure 2-4 is actually necessary? in all of the API requests I've made, the AppView always returns who you follow first, and it's been this way since the end of invite-only period worth noting that the sorting means that in lists where pagination is involved, if there were indeed profiles you follow that weren't in the first page, you just won't see them at all because it'll be sorted to the top, meanwhile you're at the bottom of the list. if this was indeed a problem, this should've been made at the AppView-level, not the app, or at the very least the sorting should not be done after flattening the paginated list. |
|
I can move 2-4 to another pr but I would like to see them fixed
I'm unsure where the data is coming from but in the app none of these lists are sorted like that.
I don't think I touched any paginated list?
Can you point me towards where is should make the change? |
|
Ah, sorry for that, I thought the starter packs had paginated queries but I forgot that's not the case. |
| @@ -151,7 +156,7 @@ const MentionList = forwardRef<MentionListRef, SuggestionProps>( | |||
| <div className="items"> | |||
| <View style={[pal.borderDark, pal.view, styles.container]}> | |||
| {items.length > 0 ? ( | |||
| items.map((item, index) => { | |||
| items.sort(sortProfiles).map((item, index) => { | |||
There was a problem hiding this comment.
sort is mutative so we can't do it here. it would have to be cloned first.
ideally, if the server response isn't appropriately sorted but we want it to be sorted, we'd place this logic closest to where the response is being parsed — i.e. in the query. mind doing that?
| @@ -1006,7 +1007,7 @@ let AutocompleteResults = ({ | |||
| } | |||
| style={{borderBottomWidth: 1}} | |||
| /> | |||
| {autocompleteData?.map(item => ( | |||
| {autocompleteData?.sort(sortProfiles).map(item => ( | |||
There was a problem hiding this comment.
same, can't mutate while rendering. doing it in the query before returning is ok.
| @@ -206,7 +207,7 @@ export function DesktopSearch() { | |||
| : undefined | |||
| } | |||
| /> | |||
| {autocompleteData?.map(item => ( | |||
| {autocompleteData?.sort(sortProfiles).map(item => ( | |||
I think putting followers at the top and bottom in certain situations leads to easier to use UI. I was initially going to do separate PRs but realized that I needed the sorting function. I applied it to some places where I think it makes sense. There may be more