Set body classes via Quicksand config
ClosedPublic
Actions

Authored by chad on Oct 1 2016, 10:57 PM.

Details

Reviewers

epriestley

Commits

rP45af6d7c0e4e: Set body classes via Quicksand config

Summary

Sends and stores additional body classes at the page level. Removes old ones, sets new ones.

Test Plan

home -> application search -> colored workboard -> config -> home with persistent chat open and minimized.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

chad updated this revision to Diff 40074.Oct 1 2016, 10:57 PM

chad retitled this revision from to Set body classes via Quicksand config.

chad updated this object.

chad edited the test plan for this revision. (Show Details)

chad added a reviewer: epriestley.

Herald added a subscriber: Korvin. · View Herald TranscriptOct 1 2016, 10:57 PM

this was much less painful than expected.

I think this won't work on the first page transition -- oldClasses will be empty, so the classes won't be removed. For example, if you start on a white page and then move to a non-white page, I'd expect it to incorrectly be white.

Using alterClass() to affect multiple classes at the same time also only sort of works by accident.

You could do this:

var oldClasses = document.body.className.split(' ');
var ii;
for (ii = 0; ii < oldClasses.length; ii++) {
  JX.DOM.alterClass(document.body, oldClasses[ii], false);
}
for (ii = 0; ii < new_classes.length; ii++) {
  JX.DOM.alterClass(document.body, new_classes[ii], true);
}

...but I don't imagine a world where that ever does anything useful. Fine to just do this, AFIAK:

document.body.className = new_classes.join(' ');

This revision is now accepted and ready to land.Oct 1 2016, 11:02 PM

im confused as to which you're suggesting

i think I still need to get the set of originally added classes via PHP

I suggest the simpler one, just because it's simpler:

document.body.className = new_data.bodyClasses.join(' ');

There's no advantage to using the more complicated one (with alterClass()) today, and I don't anticipate writing code which creates an advantage.

At one point at Facebook, there were some add/remove class calls which did do more (trigger stylesheets to load, I think?). I don't know if that code ever made it to production, but the idea was that if you did x.addClass('penguin') the code would check if the "penguin" class was defined yet and load the CSS it needed if not. If we had similar code, using alterClass() could matter, but I don't expect we ever will.

I think you shouldn't need the original classes -- assigning to document.body.className will just overwrite them, whatever they were.

i would assume that would wipe all body classes

Yeah, but that's what we want. It'll wipe the old ones and replace them with the new ones, just like we'd loaded a new page with a new <body /> tag.