🔖 streams filterbar #6358
🔖 streams filterbar #6358
Conversation
* refactor data fetching with useFetch * add filter bar to show stream categories * show athletics streams by default
| <ListSectionHeader title={title} /> | ||
| )} | ||
| sections={streams} | ||
| sections={groupStreams(filterStreams(entries, filters))} |
There was a problem hiding this comment.
Is there an obvious way we can improve on this?
| 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) => { | ||
| let enabledCategories = filters.flatMap((f: ListType) => | ||
| f.spec.selected.flatMap((s) => s.title), | ||
| ) | ||
|
|
||
| if (enabledCategories.length === 0) { | ||
| return entries | ||
| } | ||
| }, []) | ||
|
|
||
| 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'}, | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Do these really need to be fat-arrow functions? If so, can they be const at least?
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
Can we give this an interface? These fields seem magical without a type.
There was a problem hiding this comment.
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 filterStreams = (entries: StreamType[], filters: ListType[]) => { | ||
| return entries.filter((stream) => { |
There was a problem hiding this comment.
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 | ||
| } |
There was a problem hiding this comment.
Can this get pulled out of the filter function? Seems like you're not doing anything that uses stream until the very end.
There was a problem hiding this comment.
Are you asking if lines 34-40 can be extracted into their own function?
| return {title: c} | ||
| }) | ||
|
|
||
| let streamFilters: ListType[] = [ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
|
Should I break this PR up into two parts? The first would be the useFetch hook and the second would be the filter bar. |
|
It might be easier to review that way, if you don't mind the work |
|
Closing this PR in favor of the two-part approach for reviewing these changes. |
Replaced with:
These changes refactor streams list to accomplish a few ideas:
useFetch📸 Screenshots