AlexsLemonade / refinebio-examples

Example workflows for refine.bio data
https://www.refine.bio
Other
10 stars 5 forks source link

452 Fix Header + Footer + Add Homepage #457

Closed davidsmejia closed 2 years ago

davidsmejia commented 3 years ago

Purpose

Issue addressed

452

Gotchas the reviewer should know about

Remaining concerns and questions

Screen Shot 2021-07-02 at 9 38 54 AM Screen Shot 2021-07-02 at 9 39 12 AM

davidsmejia commented 3 years ago

Add link to refine.bio in how to use section

jashapiro commented 3 years ago

Navbar couldnt be dynamically included - if it were the relative links would have to be updated to go from root (which would break local dev unless you spun up a server)

I get this, but I worry that having two navbars we have to update down the line will be confusing. Can we somehow put index.html at homepage/index.html or something like that with a redirect? My other thought was some kind of js to rewrite the links, but that seems even more hackish?

davidsmejia commented 3 years ago
davidsmejia commented 3 years ago

@jashapiro I think I have addressed any feedback that you had but do you think its alright to have the homepage rendered on commit instead of the github action. I wasn't sure if they should be treated the same... also it didnt work out of the box.

davidsmejia commented 2 years ago

This looks good as an approach to getting thing inserted semi-dynamically. I'm fine with making the chances on commit rather than trying to set up actions to do it. Hopefully we are not updating all of this too often.

I have a few comments about some snakemake changes that I think could make --forceall option work, but I'm not sure whether or not you tried that already. One other possibility is to make the navbar and footer files inputs to the snakemake rules, which should force rendering of everything when those change even without forceall, but I'm less sold on that in general.

One thing I ran into and I have no idea what is going on is that the navbar isn't getting styled properly (font only, it looks like?) in Safari on a mac for the internal pages. Looks fine with Brave, but not right with Safari, and the home page looks fine in either.

Home page:

Screen Shot 2021-07-06 at 4 01 54 PM

Internal pages:

Screen Shot 2021-07-06 at 4 02 01 PM

Looks like the google font was only being included with the changes I made for the homepage.. If it had ever appeared correctly before it was because it was cached in your browser. Good catch I made some changes to fix that and include the style sheet on the notebooks!

jashapiro commented 2 years ago

Kinda minor, but I noticed in refinebio-examples/03-rnaseq/pathway-analysis_rnaseq_01_ora.html that one of the image files was not loaded because the AWS object was not public. Since the rendered html file embeds images, (Rmarkdown), this failed load is now baked in to the current rendered files. Rerendering should fix it, and while I am pretty confident that should be the only file affected, we might want to think about how to check for any other cases.