Improve core "featured records" functions
Opened this issue · 4 comments
Themes are almost all replacing these with their own and it's caused duplicated work.
The features these provide should be something we can have in the core to ease the burden on themes. Minimally this is something like having options for multiple where the core/plugin function is currently single, and possibly more. A survey of what the themes are doing is probably necessary to see what we should be providing.
I documented the custom featured record functions implemented across the themes:
- Thanks, Roy: The core only has
random_featured_collection()
andexhibit_builder_random_featured_exhibit()
, which display a single record of their respective types. Thanks, Roy’s custom function allows for multiple collections and exhibits to be displayed. - Big Picture, Center Row: Displays featured records in a slider, so allows for both multiple collections and exhibits to display, but also in a partial with custom markup to be applied to all records.
- Foundation: Provides 3 areas for displaying a featured record. The theme configuration allows the site admin to designate what record type goes in each section. These settings are passed to the custom random featured function, which then find the respective single partial to render.
- Freedom: Same as Foundation, also enables a featured timeline.
- The Daily: Displays all available featured records for items, collections, and exhibits. Uses default
single.php
for rendering.
So to summarize, the uses cases typically want to be able to pass a record type, number of records to retrieve, and define a partial to use. In addition, as the current random_featured_items()
function also allows to select records with images, it might be useful for the user to be able to define a preferred thumbnail size. Currently random_featured_items()
defaults to "fullsize".
I've drafted random_featured_records()
on the random-featured-records
branch. There are branches with the same name in Big Picture, Foundation, and Center Row.
Summarizing some thoughts on random_featured_records:
$overrides
is probably more clearly just called$partials
(and also see below for discussion on single vs. multiple call)$hasImage
isn't getting passed through to the query, but this leads to a bigger question... would we prefer this to be a more generic function and let theme writers just pass the query directly, so they could change it to a random selection of all items, a non-random selection, to require images or not, or do some totally different query? I think that's probably a good idea... though we'd probably want to name the function something slightly different...- As currently written this is skipping one of the most important areas where there were issues between the themes' implementations: handling presence/absence of Exhibit Builder. Probably the best way forward on this is to convert the static list of partials to something that just includes the core records/partials, and is passed to a filter, where plugins can modify the list to add support for their records. Trying to print the featured records for a record type not in the list would just be a no-op and return nothing, getting us handling of plugins being active/inactive/uninstalled "for free" and also allowing extension to other plugins (Geolocation? Timeline?)
- Similarly the call for the exhibit builder filter event probably shouldn't live here, instead there should be some new filter for this function's output... probably just one filter that passes along all the parameters, to let a filtering plugin decide if it wants to take action by looking at the type, or whatever else
- The other major question I have is: should this be rewritten to just return record display for one record type at a time, requiring multiple calls if a theme wants to show multiple record types together? It's simpler to write and use the function if it's single: parameters that here need to be type-indexed arrays or nested to an extra level could be simple scalars or one-dimensional arrays. And the themes are pretty sharply divided in how they call this kind of function, with many using separate calls already, to display the records in separate areas, with separate headings, etc.
would we prefer this to be a more generic function and let theme writers just pass the query directly, so they could change it to a random selection of all items, a non-random selection, to require images or not, or do some totally different query? I think that's probably a good idea... though we'd probably want to name the function something slightly different...
I'm open to this. I can definitely see more uses for this outside of a "featured" context. Maybe something as simple as display_records()
?
The other major question I have is: should this be rewritten to just return record display for one record type at a time, requiring multiple calls if a theme wants to show multiple record types together? It's simpler to write and use the function if it's single: parameters that here need to be type-indexed arrays or nested to an extra level could be simple scalars or one-dimensional arrays. And the themes are pretty sharply divided in how they call this kind of function, with many using separate calls already, to display the records in separate areas, with separate headings, etc.
Yeah, I think I'm leaning towards something more straightforward like that. It would probably make considerations for displaying plugin record types easier as well.
2413163 responds to the latest feedback. It's been renamed display_records()
, and takes a record type, query parameters for get_records()
, a partial path, an array of params to pass to the partial, and a limit of records to retrieve. Center Row, Big Picture, and Foundation have updated implementations to look at as well.