Skip to content

🔖 streams filterbar #6358

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

🔖 streams filterbar #6358

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 115 additions & 76 deletions source/views/streaming/streams/list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {timezone} from '@frogpond/constants'
import * as c from '@frogpond/colors'
import {ListSeparator, ListSectionHeader} from '@frogpond/lists'
import {NoticeView, LoadingView} from '@frogpond/notice'
import {FilterToolbar, ListType} from '@frogpond/filter'
import {StreamRow} from './row'
import toPairs from 'lodash/toPairs'
import groupBy from 'lodash/groupBy'
Expand All @@ -12,7 +13,7 @@ import type {Moment} from 'moment-timezone'
import {toLaxTitleCase as titleCase} from '@frogpond/titlecase'
import type {StreamType} from './types'
import {API} from '@frogpond/api'
import {fetch} from '@frogpond/fetch'
import {useFetch} from 'react-async'

const styles = StyleSheet.create({
listContainer: {
Expand All @@ -23,99 +24,137 @@ const styles = StyleSheet.create({
},
})

export const StreamListView = (): JSX.Element => {
let [error, setError] = React.useState<Error | null>(null)
let [loading, setLoading] = React.useState(true)
let [refreshing, setRefreshing] = React.useState(false)
let [streams, setStreams] = React.useState<
Array<{title: string; data: StreamType[]}>
>([])
let groupStreams = (entries: StreamType[]) => {
let grouped = groupBy(entries, (j) => j.$groupBy)
return toPairs(grouped).map(([title, data]) => ({title, data}))
}

React.useEffect(() => {
try {
getStreams().then(() => {
setLoading(false)
})
} catch (error) {
if (error instanceof Error) {
setError(error)
} else {
setError(new Error('unknown error - not an Error'))
}
return
let filterStreams = (entries: StreamType[], filters: ListType[]) => {
return entries.filter((stream) => {
Comment on lines +32 to +33
Copy link
Member

@rye rye Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entries could have a more descriptive name like streamEntries or streams. streams.filter((stream) => { /* */ }) is a nice pattern imo.

let enabledCategories = filters.flatMap((f: ListType) =>
f.spec.selected.flatMap((s) => s.title),
)

if (enabledCategories.length === 0) {
return entries
}
Comment on lines +34 to 40
Copy link
Member

@rye rye Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this get pulled out of the filter function? Seems like you're not doing anything that uses stream until the very end.

Copy link
Member Author

@drewvolz drewvolz Sep 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you asking if lines 34-40 can be extracted into their own function?

}, [])

let refresh = async (): Promise<void> => {
setRefreshing(true)
await getStreams(true)
setRefreshing(false)
}
return enabledCategories.includes(stream.category)
})
}

let getStreams = async (
reload?: boolean,
date: Moment = moment.tz(timezone()),
) => {
let dateFrom = date.format('YYYY-MM-DD')
let dateTo = date.clone().add(2, 'month').format('YYYY-MM-DD')

let data = await fetch(API('/streams/upcoming'), {
searchParams: {
sort: 'ascending',
dateFrom,
dateTo,
},
delay: reload ? 500 : 0,
}).json<Array<StreamType>>()

data = data
.filter((stream) => stream.category !== 'athletics')
.map((stream) => {
let date: Moment = moment(stream.starttime)
let dateGroup = date.format('dddd, MMMM Do')

let group = stream.status.toLowerCase() !== 'live' ? dateGroup : 'Live'

return {
...stream,
// force title-case on the stream types, to prevent not-actually-duplicate headings
category: titleCase(stream.category),
date: date,
$groupBy: group,
}
})

let grouped = groupBy(data, (j) => j.$groupBy)
let mapped = toPairs(grouped).map(([title, data]) => ({title, data}))

setStreams(mapped)
}
let useStreams = (date: Moment = moment.tz(timezone())) => {
let dateFrom = date.format('YYYY-MM-DD')
let dateTo = date.clone().add(2, 'month').format('YYYY-MM-DD')

return useFetch<StreamType[]>(
API('/streams/upcoming', {
sort: 'ascending',
dateFrom,
dateTo,
}),
{
headers: {accept: 'application/json'},
},
)
}
Comment on lines +27 to +60
Copy link
Member

@rye rye Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these really need to be fat-arrow functions? If so, can they be const at least?

Copy link
Member Author

@drewvolz drewvolz Sep 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can definitely be const. Is there a reason you had in mind to not make them be bound this way?


let keyExtractor = (item: StreamType) => item.eid
export const StreamListView = (): JSX.Element => {
let {data = [], error, reload, isPending, isInitial, isLoading} = useStreams()
Copy link
Member

@rye rye Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we give this an interface? These fields seem magical without a type.

Copy link
Member Author

@drewvolz drewvolz Sep 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does look magical up-front but I guarantee that the types are all pretty clear since this now uses react-async's useFetch.

The type info on line 63 for useStreams (or up above on line 46) shows the following:

let useStreams: (date?: Moment) => AsyncState<StreamType[], FetchRun<StreamType[]>>

and hovering over each of these types reflects the type information that useFetch's state provides us

let data: StreamType[]
let error: Error | undefined
let reload: () => void
let isPending: boolean
let isInitial: false
let isLoading: boolean


let renderItem = ({item}: {item: StreamType}) => <StreamRow stream={item} />
let [filters, setFilters] = React.useState<ListType[]>([])

if (loading) {
return <LoadingView />
}
let entries = React.useMemo(() => {
return data.map((stream) => {
let date: Moment = moment(stream.starttime)
let dateGroup = date.format('dddd, MMMM Do')

let group = stream.status.toLowerCase() !== 'live' ? dateGroup : 'Live'

return {
...stream,
// force title-case on the stream types, to prevent not-actually-duplicate headings
category: titleCase(stream.category),
date: date,
$groupBy: group,
}
})
}, [data])

React.useEffect(() => {
let allCategories = data.flatMap((stream) => titleCase(stream.category))

if (allCategories.length === 0) {
return
}

let categories = [...new Set(allCategories)].sort()
let filterCategories = categories.map((c) => {
return {title: c}
})

let streamFilters: ListType[] = [
Copy link
Member

@rye rye Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to tighten ListType or rename it to something like FilterType? It seems like we're using a type called List for a ... list of filters? Shouldn't it just be a list of filters?

Copy link
Member Author

@drewvolz drewvolz Sep 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you asking if ListType can be ListFilterType? It probably could. This is coming from modules/filter/types.ts where we define a whole scope of different FilterType definitions.

export type ToggleType = {
	type: 'toggle'
	key: string
	enabled: boolean
	spec: ToggleSpecType
	apply: ToggleFilterFunctionType
}

export type PickerType = {
	type: 'picker'
	key: string
	enabled: true
	spec: PickerSpecType
	apply: PickerFilterFunctionType
}

export type ListType = {
	type: 'list'
	key: string
	enabled: boolean
	spec: ListSpecType
	apply: ListFilterFunctionType
}

export type FilterType = ToggleType | PickerType | ListType

{
type: 'list',
key: 'category',
enabled: true,
spec: {
title: 'Categories',
options: filterCategories,
selected: filterCategories,
mode: 'OR',
displayTitle: true,
},
apply: {key: 'category'},
},
]
setFilters(streamFilters)
}, [data])

if (error) {
return <NoticeView text={`Error: ${error.message}`} />
return (
<NoticeView
buttonText="Try Again"
onPress={reload}
text={`A problem occured while loading the streams. ${error.message}`}
/>
)
}

const header = (
<FilterToolbar
filters={filters}
onPopoverDismiss={(newFilter) => {
let edited = filters.map((f) =>
f.key === newFilter.key ? newFilter : f,
)
setFilters(edited as ListType[])
}}
/>
)

return (
<SectionList
ItemSeparatorComponent={ListSeparator}
ListEmptyComponent={<NoticeView text="No Streams" />}
ListEmptyComponent={
isLoading ? (
<LoadingView />
) : filters.some((f: ListType) => f.spec.selected.length) ? (
<NoticeView text="No streams to show. Try changing the filters." />
) : (
<NoticeView text="No streams." />
)
}
ListHeaderComponent={header}
contentContainerStyle={styles.contentContainer}
keyExtractor={keyExtractor}
onRefresh={refresh}
refreshing={refreshing}
renderItem={renderItem}
keyExtractor={(item: StreamType) => item.eid}
onRefresh={reload}
refreshing={isPending && !isInitial}
renderItem={({item}: {item: StreamType}) => <StreamRow stream={item} />}
renderSectionHeader={({section: {title}}) => (
<ListSectionHeader title={title} />
)}
sections={streams}
sections={groupStreams(filterStreams(entries, filters))}
Copy link
Member Author

@drewvolz drewvolz Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an obvious way we can improve on this?

style={styles.listContainer}
/>
)
Expand Down