Skip to content
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.

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

Use showcase on splash #901

Merged
merged 10 commits into from
Jun 13, 2024
Merged

Use showcase on splash #901

merged 10 commits into from
Jun 13, 2024

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Jun 6, 2024

preview

image

Description of proposed changes

Moving cards from individual sections to a shared showcase defined by a single YAML file.

For later

  • surface more info for each card (ref)

Related issue(s)

#849

Checklist

@victorlin victorlin self-assigned this Jun 6, 2024
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--9rlx1f June 6, 2024 17:59 Inactive
This was referenced Jun 6, 2024
@victorlin victorlin marked this pull request as ready for review June 6, 2024 18:09
@victorlin victorlin requested a review from a team June 6, 2024 18:09
@victorlin victorlin mentioned this pull request Jun 6, 2024
4 tasks
genehack
genehack previously approved these changes Jun 7, 2024
static-site/src/components/ListResources/Showcase.tsx Outdated Show resolved Hide resolved
static-site/src/components/ListResources/Showcase.tsx Outdated Show resolved Hide resolved
Base automatically changed from victorlin/splash-refactor to master June 7, 2024 17:14
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

This is really cool to see!

It makes me think that visual indicators to distinguish {core,groups,community} and {dataset,narrative} will be really useful¹. Similar in spirit to Nextclade's badges. (All of which are out of scope for this PR -- without one of us taking on the role of a UI designer these interfaces will have to develop naturally over time.)

¹ The card styles partially convey this, but not in an understandable way to users.

static-site/src/components/ListResources/Showcase.tsx Outdated Show resolved Hide resolved
static-site/src/components/ListResources/Showcase.tsx Outdated Show resolved Hide resolved
static-site/src/components/ListResources/types.ts Outdated Show resolved Hide resolved
static-site/src/components/splash/index.jsx Show resolved Hide resolved
static-site/src/components/splash/index.jsx Show resolved Hide resolved
@victorlin victorlin force-pushed the victorlin/use-showcase-on-splash branch from 644ea28 to 0dc321a Compare June 10, 2024 18:24
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 10, 2024 18:25 Inactive
@victorlin victorlin force-pushed the victorlin/use-showcase-on-splash branch from 0dc321a to 98f4616 Compare June 10, 2024 18:27
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 10, 2024 18:28 Inactive
@victorlin victorlin force-pushed the victorlin/use-showcase-on-splash branch from 98f4616 to 7b99c6a Compare June 10, 2024 18:50
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 10, 2024 18:50 Inactive
@victorlin victorlin force-pushed the victorlin/use-showcase-on-splash branch from 7b99c6a to eefb79e Compare June 10, 2024 20:32
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 10, 2024 20:32 Inactive
This allows the text to be customized for non-filter showcases.
@victorlin victorlin force-pushed the victorlin/use-showcase-on-splash branch from eefb79e to 66b395f Compare June 10, 2024 22:06
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 10, 2024 22:06 Inactive
Copy link
Member Author

Choose a reason for hiding this comment

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

Design thinking

With this PR, the Showcase component is used in 2 places:

  1. Splash (/)
  2. ListResources (/pathogens)

There are two features proposed for usage on Splash:

  1. Surfacing an abstract (ref)
  2. Visual indicators for core/groups/community and dataset/narrative (ref)

These are great ideas and I see them as in-scope of #849's completion.

From an implementation perspective, it's helpful to consider which of these features also apply to ListResources. (1) maybe, but most likely not (2).

Given the existing and upcoming divergence in features, I decided to make the entire individual card component customizable by the context, implemented as 25b545d + 7c2d301:

<SetSelectedFilterOptions.Provider value={setSelectedFilterOptions}>
<Showcase cards={showcaseCards} CardComponent={FilterShowcaseTile} />
</SetSelectedFilterOptions.Provider>

<Showcase cards={cards} CardComponent={UrlShowcaseTile} />

This is a larger refactor, but it should make those design ideas easier to implement.

@victorlin victorlin dismissed genehack’s stale review June 10, 2024 22:21

significant new changes pushed

@victorlin victorlin requested a review from genehack June 10, 2024 22:21
@victorlin victorlin force-pushed the victorlin/use-showcase-on-splash branch from 66b395f to e6fa8aa Compare June 10, 2024 23:26
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 10, 2024 23:26 Inactive
This allows the showcase to be used in other contexts.

The most notable change is the separation of the filtering card
component (specific to ListResources) from the reusable parts of the
Showcase component.
victorlin and others added 4 commits June 10, 2024 16:57
Start by copying the current featured listings from the different
sections on Splash.
These are shown in the newly added showcase.
Reflects changes from the previous two commits.
Making some of these dataset URLs explicit somewhat goes against the
slack discussion linked in 3edcf03 but
now it's easy to change the cards - something we'll hopefully do
frequently - that explicit links, and the more detailed titles they
allow, are preferable.

Co-authored-by: James Hadfield <hadfield.james@gmail.com>
@victorlin victorlin force-pushed the victorlin/use-showcase-on-splash branch from e6fa8aa to a19026e Compare June 11, 2024 00:00
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 11, 2024 00:00 Inactive
Here I went through to try to surface non-core datasets. I also tried to temper down the language. It's more approachable to just say "H5N1 cattle outbreak" instead of "Avian influenza (cattle flu genome)" or "Ebola in the DRC" rather than "DRC Ebola (2018-19)". I also switched to capitalization consistent with mid-sentence usage, eg "mpox" rather than "Mpox" as per our capitalization on the /pathogens page.
@trvrb trvrb temporarily deployed to nextstrain-s-victorlin--9rlx1f June 12, 2024 19:18 Inactive
@trvrb trvrb temporarily deployed to nextstrain-s-victorlin--9rlx1f June 12, 2024 19:23 Inactive
@trvrb
Copy link
Member

trvrb commented Jun 12, 2024

I updated tiles further mainly to include more examples and to simplify text. I also did a small update to the styling to make text fit better. Here's before 4bc2acc:

before

And here's after:

updated

@trvrb
Copy link
Member

trvrb commented Jun 12, 2024

Nice work here @victorlin! With the small tweaks above, I'm happy for this to be merged into master. I like the staging here where revamp of the linkouts can be done in a subsequent PR ala #902.

victorlin and others added 2 commits June 12, 2024 14:39
Not all were copied, resulting in a mix of imports from Showcase and
Cards. Copy the rest to allow further Showcase-specific customization.
This pushes the black text box up and to the left to leave more space for text.
@victorlin victorlin force-pushed the victorlin/use-showcase-on-splash branch from 4bc2acc to 52e964e Compare June 12, 2024 21:43
@victorlin
Copy link
Member Author

Thanks for pushing the tweaks @trvrb. I've replaced 4bc2acc with 9f72478...52e964e so that the styling adjustments don't affect the cards on /groups.

@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 12, 2024 21:44 Inactive
@victorlin
Copy link
Member Author

I'll plan to merge this tomorrow, leaving another day for reviews.

In terms of implementation, I'm confident in most changes except 1414985 may prompt some feedback. After some thinking in #849 (comment) and some design drafting today, I've a sense that we'll eventually want enough deviation where it doesn't make sense to re-use the same Showcase component between splash and /pathogens. But this is still the easiest approach at the moment, and the preview looks good.

jameshadfield
jameshadfield approved these changes Jun 13, 2024
static-site/src/components/Showcase/index.tsx Show resolved Hide resolved
The arrow should be the same size across all usages, not inferred by
parent font size.
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--9rlx1f June 13, 2024 17:39 Inactive
@victorlin victorlin merged commit 0066127 into master Jun 13, 2024
7 checks passed
@victorlin victorlin deleted the victorlin/use-showcase-on-splash branch June 13, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants