Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Implement Galleries as JSON+JS for all themes #1765

Closed
wants to merge 21 commits into
from

Conversation

Projects
None yet
3 participants
Owner

ralsina commented May 28, 2015

Working on #1764 and #1623

Review on Reviewable

ralsina added some commits May 28, 2015

@ralsina ralsina changed the title from [WIP] Refactor galleries again to [WIP] Implement Galleries as JSON+JS for all themes May 29, 2015

@ralsina ralsina referenced this pull request May 29, 2015

Closed

Make galleries (and maybe slides?) work with base theme #1764

2 of 3 tasks complete

ralsina added some commits May 29, 2015

@ralsina ralsina changed the title from [WIP] Implement Galleries as JSON+JS for all themes to Implement Galleries as JSON+JS for all themes May 29, 2015

Owner

ralsina commented May 29, 2015

@Aeyoun this adds JS galleries to base... is that ok with you? Base is your baby :-)

The main goal is to let all the other themes just inherit gallery.tmpl and not have to tweak it.
For example, consider how it looks here https://themes.getnikola.com/v7/lanyon/galleries/demo/index.html ... with these changes it "Just Works".

@da2x da2x and 1 other commented on an outdated diff May 29, 2015

docs/creating-a-theme.txt
<head>
- <meta name="viewport" content="width=device-width, initial-scale=1.0">
- ${bootstrap.html_head()}
- <%block name="extra_head">
- </%block>
- ${extra_head_data}
+ <meta charset="utf-8">
+ <meta name="description" content="This is a demo site for Nikola.">
+ <meta name="viewport" content="width=device-width">
+ <title>My Nikola Site | My Nikola Site</title>
+
+ <link href="assets/css/rst.css" rel="stylesheet" type="text/css">
@da2x

da2x May 29, 2015

Contributor

Type is not needed for text/css.

@ralsina

ralsina May 29, 2015

Owner

I thought some validator or other complained if it was not there?

@ralsina

ralsina May 29, 2015

Owner

In any case, I have no idea why this file is in this diff. It's exactly the same in master :-(

ralsina added some commits Jun 2, 2015

Owner

Kwpolska commented Jun 5, 2015

  1. The way you load jQuery and flowr is hacky, and will not make USE_CDN=False users happy; it would be easier to just put the JS in the usual place for all pages (besides, I already sneaked some JS into base when no-one looked)
  2. The galleries are now completely inaccessible to people with JavaScript disabled; previous versions had graceful fallback with the images still visible (just add the <noscript> bit back in)
  3. Why does base have its own render function? Why are there no such functions in bootstrap and bootstrap3? What would happen if you did not put the function there?
Owner

ralsina commented Jun 5, 2015

@Kwpolska

  1. You mean putting jquery in base and using USE_CDN to load one or the other? Done
  2. I don't much mind that usecase. Sure I can add it, but the web in general is inaccessible with JS disabled. In this specific case, sure, why not. Done
  3. Because base didn't have it before, bootstrap* did: https://github.com/getnikola/nikola/blob/refactor-galleries-gain/nikola/data/themes/bootstrap/templates/gallery.tmpl#L49

ralsina added some commits Jun 9, 2015

wat
Owner

ralsina commented May 14, 2017

I will take another shot at this, but this bitrot way too much.

@ralsina ralsina closed this May 14, 2017

@Kwpolska Kwpolska deleted the refactor-galleries-gain branch May 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment