Display repo and maintainers as byline #821
Conversation
This byline includes both repository and maintainers where available in metadata. This replaces the use of maintainers in footer.
|
The UI here looks great ✨, and I ❤️ that we're better highlighting build maintainers with a byline. |
|
I'm 100% behind this. For completeness, see also this PR from July: #761
|
|
Thanks Tom and thanks James! I've just requested a review from you @jameshadfield. I suspect some of my JS use here could be improved. |
|
Thanks @trvrb. I'd like to split this into two separate sections:
I'll play with both of these this week. Note that this is not blocking augur development or release in any way. |
I think it'd be nice to remove |
See commit message of nextstrain/nextstrain.org@fc4aee3 for rationale
Auspice (2.*) allows the `getAvailable` API request to define `buildUrl`s for datasets. These were displayed in the sidebar, above the choose-dataset dropdowns. This commit removes the sidebar display of this, as it's been super-seeded by the byline. The commit also sets the `metadata.buildUrl` to be that defined in the `getAvailable` API response _if_ it has not been set via the dataset JSON, as the latter is to be considered the "canonical" way of defining this value.
Refactors the byline into its own React Component. In general, I think this helps reduce interpretability of the code. No changes in UI. Minimal changes to implementation, related to making this a functional component which are simpler than react classes (and react is generally moving away from classes).
Feels much cleaner to me, but this is highly subjective!
The "should i render" boolean approach is replaced with returning a react component when it's determined that something should be rendered, else returning `null` as a fallthrough at the end of the function (null is better than `<span/>`). The `renderLink` function is replaced by a component, which gets passed props rather than an object. Looks like lots of changes but not that many really!
|
@trvrb i've made a few changes & think this is pretty good to go once we decide on the key name for the dataset JSON (currently I've changed the font size & weight as it looked much heavier on my setup than yours, but this needs testing (feel free to get rid of this commit if you want).
This is specified in the v2 auspice API here: https://nextstrain.github.io/auspice/server/api#charon-getavailable. It won't be used if a |
The originally intention was to have the overall weight (including size, color and font weight) of byline to be about halfway between title and info. I see that the weight 700 wasn't working. This looks a little better to my eye while still incorporating direction implemented by @jameshadfield.
|
Thanks James! Changes look great. I agree byline works better as a separate React component. The only suggestion I had was to split the difference with aesthetic changes. I just pushed a small change there. I'm happy merging, but I do think we should wait on decision on |
Agreed. No huge hurry here as there are no JSONs which have it yet! 😉
Great! We already request weight 500 fonts so no extra overhead here. |
This PR aims to do two things:
This looks like this for the Zika Tutorial build:
This is what this looks like for the Ebola DRC build:
(There are some more mocks in Slack here: https://bedfordlab.slack.com/archives/C116R9PR7/p1564668779044000)
This would require groups to use custom GitHub profile photos if they want their logo shared.
This uses
metadata.repository, but could easily be changed tometadata.buildUrlor something else (nextstrain/augur#408 (comment)).I'd imagine this working with existing community functionality in that if
metadata.repositoryis not specified and charongetAvailablehas sharedbuildUrlwith auspice, thenmetadata.repositorywould use the URL passed in from community.metadata.repositorywould override passed in communitybuildUrlif both exist.