Port new resource-listing “cards” UI for use to display datasets on /groups and /community
Opened this issue · 9 comments
Work breakdown:
- update
useDataFetch
code to pull groups data when given appropriate arguments - update
useDataFetch
code to transform groups data into expected shape - update
/groups
page to useResourceList
component -
convert narratives into simple list as suggested - add link from group name to group page
- add custom logos when we have them
- it doesn't appear we have these in any centralized way
- is this worth doing per-group
fetch()
calls during the card render to hit thegetSourceInfo
endpoint to retrieve theavatar
field and inject it over the default image? (feels expensive.) - "Then maybe best to extend metaSources to return group logos in addition to datasets/narratives" (context)
- The way group logos are stored is really not conducive to accessing them in this way. I think what I'm going to do instead is repress the logo for anything that isn't
nextstrain
- add resource list to single-group pages
- should be mostly lift and shift from
/groups
- should be mostly lift and shift from
- add resource list to /community
- add last-modified dates to group datasets
- maybe/probably out of scope for this issue and better handled on its own?
FYI I'm planning to make some edits to static-site/src/components/ListResources/Showcase.jsx proposed in #849 (comment).
We may run into conflicts here, but I suspect that as long as you use the Showcase
component just like this in the other pages, it should be trivial to resolve.
@genehack happy to talk through any design questions you have here (as they arise). Perhaps a couple of points as to what I was thinking:
- The current
<ListResources>
component gets its data from a hardcoded API call. The current groups/community pages uses thecharon/getAvailable
API. My suggestion is to allow the<ListResources>
component to use thosecharon/getAvailable
APIs, either by expandinguseDataFetch
or by injecting custom data fetch functions to the component. - Narratives aren't currently displayed in
<ListResources>
. They totally can be, but lots of UI decisions there. As an intermediate we could replace the existing narrative table with a simple HTML list, which I think would be just fine.
@genehack happy to talk through any design questions you have here
Thanks, I was gonna use our 1:1 time today to get a bootstrapping brain dump from you on this. 😁
Noting that Heroku deployment is currently blocked on making eslint
aware of TypeScript.
Noting that Heroku deployment is currently blocked on #905.
I mean, we could just fix the linting errors highlighted by the failing build step (which you can run locally as well), which'll let this PR be built as a Heroku review app and even merged. Not a great long term state, but doesn't seem like something that should block this work.
Noting that Heroku deployment is currently blocked on #905.
I mean, we could just fix the linting errors highlighted by the failing build step (which you can run locally as well), which'll let this PR be built as a Heroku review app and even merged. Not a great long term state, but doesn't seem like something that should block this work.
One of the linting errors is ESLint claiming the variable in a function type isn't being used — that is, the linting errors are because we're linting Typescript code with a linter that doesn't understand Typescript.
I don't know how to fix that without fixing the linting — maybe I'm missing some angle?
You can fix the warning by adding the dependency to useEffect
and suppress the two errors arising from the lack of TypeScript parsing.
diff --git a/static-site/src/components/ListResources/index.tsx b/static-site/src/components/ListResources/index.tsx
index 467dec9d..d0443f97 100644
--- a/static-site/src/components/ListResources/index.tsx
+++ b/static-site/src/components/ListResources/index.tsx
@@ -123,6 +123,8 @@ interface ListResourcesResponsiveProps {
defaultGroupLinks: boolean
groupDisplayNames: GroupDisplayNames
showcase: Card[]
+ // <https://github.com/nextstrain/nextstrain.org/issues/905>
+ // eslint-disable-next-line no-unused-vars
parserCallBackFxn: (response: Response) => Promise<{ pathPrefix: string; pathVersions: PathVersions; }>;
}
diff --git a/static-site/src/components/ListResources/useDataFetch.ts b/static-site/src/components/ListResources/useDataFetch.ts
index 49b9634e..372d30a9 100644
--- a/static-site/src/components/ListResources/useDataFetch.ts
+++ b/static-site/src/components/ListResources/useDataFetch.ts
@@ -20,6 +20,8 @@ export function useDataFetch(
versioned: boolean,
defaultGroupLinks: boolean,
groupDisplayNames: GroupDisplayNames,
+ // <https://github.com/nextstrain/nextstrain.org/issues/905>
+ // eslint-disable-next-line no-unused-vars
parserCallBack: (response: Response) => Promise<{ pathPrefix:string, pathVersions:PathVersions }>
) : {groups: Group[] | undefined, dataFetchError: boolean | undefined} {
const [groups, setGroups] = useState<Group[]>();
@@ -66,7 +68,7 @@ export function useDataFetch(
}
fetchAndParse();
- }, [sourceId, versioned, defaultGroupLinks, groupDisplayNames]);
+ }, [sourceId, versioned, defaultGroupLinks, groupDisplayNames, parserCallBack]);
return {groups, dataFetchError}
}
You can fix the warning by adding the dependency to
useEffect
and suppress the two errors arising from the lack of TypeScript parsing.
Added the dep to useEffect
; suppressing the two errors feels counter-productive, as I'll just have to take the disable statements back out once the linting fix lands.