People list page halp

[kissane]

AS FORETOLD by every smart person I have failed to scrape together enough photos to make our giant people list look good, so although we have a bunch and I can continue to add them, I need to admit failure on that piece so I can focus my attentions more broadly.

@ryanpitts, how hard do you think it would be to just suppress the damn images (headshots and placeholders, the whole div) on the People list until we can feed in more headshots? I think this can be done via .src-promo a img with a display:none but I don't know how dramatic it is to make changes to the CSS.

[beep]

Apologies for CSSealioning in here, but I might recommend adding an extra class to that list (e.g., class="src-promo list-people"), and classing against that. (.list-people img { display: none; })

</cssealion>

[ryanpitts]

Yep, this seems like a good solution. @beep one quick question re grunt compiling. I took your suggestion and commented out the criticalcss task in Gruntfile.js, but when I run grunt now it deletes the file critical-home.css. Does that seem ... expected?

[kissane]

THANK YOU @BEEP I have literally no idea what I am doing

[beep]

@ryanpitts Yeah, sorry: that’s the grunt clean task hard at work, removing old dist files. I think you can safely comment that out as well, especially if you’re not using criticalcss right now.

[beep]

THANK YOU @BEEP I have literally no idea what I am doing

@kissane i mean, same, you’ve seen my work right

[kissane]

@beep ethan.

[beep]

(happy to help! let me know if there’s anything else at all that i can do)

[beep]

@kissane yes hello

[kissane]

moshi moshi

[beep]

[ryanpitts]

@beep ah yup, that works great.

[ryanpitts]

@beep one more question for you, also re: this person list page. I've got a little javascript util that puts an in-page filter in place for quick searching. Example here.

It works by flipping elements' display attributes. I'm noticing on .src-promo though that there's an .src-promo:nth-child( 3n ) rule that sets margin-right: 0. So flipping display between none and list-item ends up making the list look kind of crazy. Wondering if you have any thoughts on either a way to avoid using nth-child or a way to force the ul to recalculate which items are, in fact, nth, after a filter is applied.

[beep]

@ryanpitts Oh, hm. That’s a tough one.

Short answer: sadly, nth-child isn’t robust enough to deal with class/visibility changes.

But. But! Try taking this bit out of _core.scss?

    .src-promo:nth-child( 3n ) {
        margin-right: 0;
    }

This might cause some regressions, but a quick test suggests it should be fine—still, I’d suggest looking at it in a few contexts before deploying widely. But if it works, that’d clear up the display weirdness you were seeing on filter.

[kissane]

Sorry dudes. Sorry sorry.

[beep]

(I have to head out to dinner now, but I’ll check back in first thing tomorrow. If any CSS/JS/layout issues pop up, please don’t hesitate to assign me some issues.)

[ryanpitts]

@beep hate to ask, but I'm running into some problems adding custom javascript to a page that relies on jQuery to fire. The list filters on the People and Organizations list pages work sometimes, but not all the time.

Initially I tried adding the js to each of those pages independently, but that regularly gave me a $ is not defined error (even inside a document.ready), which I assume means main.js wasn't finished loading. So I reconfigured listfilter.js a little bit and added it to the compile step here https://github.com/OpenNews/opennews-source/commit/e839039598de24353d53e7afebfbd5f811fcc269. It looks for the presence of a few vars on the page, and if it finds them, constructs the filter.

This works sometimes, but not all the time. No js errors when it fails, just ... nothing. So I think it might be a timing issue? Have any thoughts about adding custom js to pages like this?

[beep]

@ryanpitts Hey, I’m glad you asked. (And I should’ve been more proactive in asking about Source’s custom JS needs. Apologies.)

Initially I tried adding the js to each of those pages independently, but that regularly gave me a $ is not defined error (even inside a document.ready), which I assume means main.js wasn't finished loading.

Yep, I think that’s right. We’re deferring the load of main.js (and only in “advanced” browsers), so timing issues could definitely pop up.

This works sometimes, but not all the time. No js errors when it fails, just ... nothing. So I think it might be a timing issue? Have any thoughts about adding custom js to pages like this?

One suggestion: maybe try something like what I pushed to 3e3bb64926e0e3d13e9ce8ad30139cf1395ab2ff? (In the 109-defer branch.) Take a look at tmpl/repo-featured.php for an example, but basically, that function would let you include something like this in the head of your page:

<script>
SRC.utils.defer( function() {
    // Custom JS that depends on jQuery could go here
    console.log( "Hey, jQuery’s loaded!" );
} );
</script>

And you could stick any custom code inside that function, and it should only execute once window.jQuery is available.

This is a pretty slapdash patch, and could definitely be neater. But it should help, I think?

(Let me know if it doesn’t, though?)

[ryanpitts]

@beep Oh interesting! So in theory I could:

1) take listfilter.js back out of the compiled js 2) make a new listfilter.js file, wrapping the whole thing in SRC.utils.defer, and store it separately in the static dir 3) call in that file only on the two pages where we're using filtering

That sound right?

[beep]

@ryanpitts Oh! I might’ve misunderstood slightly.

So, yes: that’s definitely an option, and a good one. And if that feels right to you, run with it.

Another thing to consider: you might look at how we’re currently loading in Disqus’ JS on pages that require it. Namely, we’re:

1. Setting a few variables,
2. If a certain element is present, then we
   1. Load in the external JavaScript file, and
   2. Execute some code once the file’s loaded.

Since that tiny JS file is included in main.js, the check runs on each page. But the external code is loaded/initialized only on pages that meet certain conditions.

The defer method seems like it’d be best for, say, article pages that need to run custom JS when jQuery’s available. But loading an external file seems like an approach that might work for your list filters.

Dunno. That’s my fairly sleepy take, at least—what do you think?

[ryanpitts]

@beep ahhhhhh, yes that was actually the file I was looking at while concocting this version of listfilter.js, but I completely overlooked that SRC.utils.loadJS was doing a deferred load. I'll give that method a shot. Thanks!

[ryanpitts]

eh, I guess I was actually looking at count-disqus.js, but anyway. Seems like this should work.

[beep]

@ryanpitts Ah, yeah, that’s basically the same thing. Hope it helps!

Also, I’m signing off for the night, but I’m happy to take a crack at this (or anything else) in the morning. Let me know if I can help!

[kissane]

No checking GitHub at niiiiiiiight @beep. <3

[ryanpitts]

HA, I think that did it! Thank you so much, @beep. If you have a moment in the morning, I'd love for another set of eyes on what I did in https://github.com/OpenNews/opennews-source/commit/20584b5cc91243b8ac810c738433736771fadcd6

Basically:

1. moved the page vars to test for into the head (rather than just before the closing body where I had them)
2. created this version of listfilter.js and stashed it in static/base/_v2/js
3. created listfilter-load.js and added that to the compile step for main.js

This approach seems to be working here and here, but would be great to have a bit of testing!

[kissane]

Looks awesome from here!

[beep]

Beautifully, beautifully done, @ryanpitts. Seems to be working great!

[beep]

No checking GitHub at niiiiiiiight @beep. <3

sorry sorry @kissane i’ll show myself out

[ryanpitts]

out of curiosity, if I'm checking for the presence of some javascript vars before firing off the load request for listfilter.js, should I be making sure those vars are defined before even initial.js gets loaded? I think I just ran into a timing issue still, although I'm not positive because it's totally working fine on next load. Here's the current head, defining those vars right at the end:

Just wondering if this ordering is something I should consider, especially going forward if we run into other needs like this.

[beep]

@ryanpitts So yeah, you have a couple options here!

1. If you want to move those vars before initial.js in the head, that’d probably clear up the timing issue.
2. If those variables are going to be the same on every page that invokes listfilter.js, I think you could safely define them right in listfilter-load.js, perhaps here-ish.

[ryanpitts]

@beep awesome, thanks! I'll make a template block in _v2/base.html that lets me inject js before initial.js loads for cases like this.

Unfortunately the vars won't quite be the same on every page where we're filtering -- like for the person list, the filterable objects need filterItemClass = '.src-promo', but on the organization list the markup is different, so we need filterItemClass = '.profile'.

[beep]

Ah, that makes sense!

(Free advice is worth every penny and all that, but I’ll sometimes introduce a module-agnostic class that a given JS plugin might hook into. Like, a JS-only .is-filter-list would let me have .src-promo.is-filter-list and .profile.is-filter-list.)

[beep]

oh good i just graduated into scriptsplaining /runs into the forest

[ryanpitts]

I’ll sometimes introduce a module-agnostic class that a given JS plugin might hook into. Like, a JS-only .is-filter-list would let me have .src-promo.is-filter-list and .profile.is-filter-list.)

Oh man, I like this much better. I'll still add a {% block head_extra_pre_initial_js %} to the base template for the future, but adding a class like that to the markup means I can just collect with jQuery and test like you're doing here, rather than my dumb typeof !== 'undefined' shenanigans.

[ryanpitts]

skrr skrr https://github.com/OpenNews/opennews-source/commit/bf381bf6a652768edc2a719e2e4b47452a6595b8

[ryanpitts]

bumping the disqus vars on the article template up before initial.js loads as well, fwiw