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

452 Fix Header + Footer + Add Homepage #457

Merged
merged 48 commits into from
Jul 15, 2021
Merged

Conversation

davidsmejia
Copy link
Contributor

@davidsmejia davidsmejia commented Apr 23, 2021

Purpose

  • Add homepage to the examples site
  • fix scaling of header/navbar
  • adds footer/fix layout
  • adds documentation for the homepage
  • rerenders all the things
  • fixes the fonts by adding the font file to be downloaded and renaming google-analytics.html -> header-includes.html

Issue addressed

#452

Gotchas the reviewer should know about

  • Imported the vendor dependencies from a cdn which I would consider "production ready".
  • The homepage has to be rendered and committed since snakemake --forceall won't trigger the rule for the homepage.

Remaining concerns and questions

  • 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)

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

@davidsmejia
Copy link
Contributor Author

Add link to refine.bio in how to use section

@jashapiro
Copy link
Member

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?

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

Since this keeps popping up in my slack reminders, I am submitting a new comment as a "review".

Overall, this looks great, but having two separate header locations to keep track of and keep updated strikes me as a problem we should solve. I had an idea in my earlier comment #457 (comment), but I don't know how workable it is...

My other question is if we want to incorporate #454 into this PR (or merge that first?)

@davidsmejia
Copy link
Contributor Author

  • add footer to homepage
  • make the footer wider and at the bottom of the doc
  • add github action to inject the header on deploy
  • align the set up and other section on the homepage

davidsmejia and others added 8 commits July 13, 2021 11:43
@davidsmejia
Copy link
Contributor Author

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
Copy link
Member

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.

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

This all looks good to me! There is the one note about rerendering for a broken image that I made earlier, but otherwise, pretty! #457 (comment)

@davidsmejia davidsmejia merged commit 9e29653 into staging Jul 15, 2021
@davidsmejia davidsmejia deleted the davidsmejia/542-homepage branch July 15, 2021 15:16
davidsmejia added a commit that referenced this pull request Jul 16, 2021
* adds homepage

* fix html head tag

* remove navbar from homepage and move to components

* align items at top

* align items start not top

* remove navbar styles

* add rule for render-homepage script

* viewport and margins

* fix image for Rmarkdown

* fix footer layout

* homepage rendered with render-homepage script

* add script that creates index.html

* force footer to render as last element in body

* index.html

* add hrefs to rewrite path

* add blurb to footer

* style the footer

* render updated footer

* rendering out the updated headers

* cleaning up whitespace

* fixing link to homepage in navbar

* clean up header

* rerender homepage with fixed logo

* fix nav spacing

* re-render notebooks with updated header/footer

* fix closing tag

* rerender homepage

* move padding but might be better to remove since the footer takes up some space

* rerender with updated footer

* initial homepage docs

* more documentation

* Update UPDATING_HOMEPAGE.md

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* Update scripts/render-homepage.R

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* relative homepage links in navbar

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* Update Snakefile

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* add index to inputs

* move google analytics to generic includes

* remove copy about force all

* render index

* use the generic header-includes file

* trigger rerender when navbar and footer change

* add font files

* remove font info

* add font defs

* rerender with correct fonts

* change back to google-analytics.html

* rerender after changing the google analytics file back

* rerender because images were missing

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>
davidsmejia added a commit that referenced this pull request Jul 16, 2021
* adds homepage

* fix html head tag

* remove navbar from homepage and move to components

* align items at top

* align items start not top

* remove navbar styles

* add rule for render-homepage script

* viewport and margins

* fix image for Rmarkdown

* fix footer layout

* homepage rendered with render-homepage script

* add script that creates index.html

* force footer to render as last element in body

* index.html

* add hrefs to rewrite path

* add blurb to footer

* style the footer

* render updated footer

* rendering out the updated headers

* cleaning up whitespace

* fixing link to homepage in navbar

* clean up header

* rerender homepage with fixed logo

* fix nav spacing

* re-render notebooks with updated header/footer

* fix closing tag

* rerender homepage

* move padding but might be better to remove since the footer takes up some space

* rerender with updated footer

* initial homepage docs

* more documentation

* Update UPDATING_HOMEPAGE.md

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* Update scripts/render-homepage.R

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* relative homepage links in navbar

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* Update Snakefile

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* add index to inputs

* move google analytics to generic includes

* remove copy about force all

* render index

* use the generic header-includes file

* trigger rerender when navbar and footer change

* add font files

* remove font info

* add font defs

* rerender with correct fonts

* change back to google-analytics.html

* rerender after changing the google analytics file back

* rerender because images were missing

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>
@davidsmejia davidsmejia mentioned this pull request Jul 16, 2021
2 tasks
davidsmejia added a commit that referenced this pull request Jul 16, 2021
* adds homepage

* fix html head tag

* remove navbar from homepage and move to components

* align items at top

* align items start not top

* remove navbar styles

* add rule for render-homepage script

* viewport and margins

* fix image for Rmarkdown

* fix footer layout

* homepage rendered with render-homepage script

* add script that creates index.html

* force footer to render as last element in body

* index.html

* add hrefs to rewrite path

* add blurb to footer

* style the footer

* render updated footer

* rendering out the updated headers

* cleaning up whitespace

* fixing link to homepage in navbar

* clean up header

* rerender homepage with fixed logo

* fix nav spacing

* re-render notebooks with updated header/footer

* fix closing tag

* rerender homepage

* move padding but might be better to remove since the footer takes up some space

* rerender with updated footer

* initial homepage docs

* more documentation

* Update UPDATING_HOMEPAGE.md

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* Update scripts/render-homepage.R

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* relative homepage links in navbar

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* Update Snakefile

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

* add index to inputs

* move google analytics to generic includes

* remove copy about force all

* render index

* use the generic header-includes file

* trigger rerender when navbar and footer change

* add font files

* remove font info

* add font defs

* rerender with correct fonts

* change back to google-analytics.html

* rerender after changing the google analytics file back

* rerender because images were missing

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>

Co-authored-by: jashapiro <josh.shapiro@ccdatalab.org>
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.

None yet

2 participants