washingtonstateuniversity / WSUWP-Content-Visibility

Control the visibility of content for groups of authenticated users.
Other
1 stars 0 forks source link

[PR] Improve support for content viewer groups #10

Closed jeremyfelt closed 8 years ago

jeremyfelt commented 8 years ago

When complete this will provide a base for plugins and themes to handle the search and assignment of content viewer groups.

Fixes #3

jeremyfelt commented 8 years ago

Ok. I think we're getting close. 😄

@philcable can you check things out and review? No merge yet, but soon.

Of note:

philcable commented 8 years ago

@jeremyfelt Looks good and tested out well! I might look over it one more time just for good measure.

jeremyfelt commented 8 years ago
jeremyfelt commented 8 years ago

Since the last review:

In the last 5 days:

@philcable Ready for another round of review! I'll explain the changes in person tomorrow. I may do one or two README commits before then.

philcable commented 8 years ago

@jeremyfelt This is looking good! Works great, and so far I haven't been able to break anything!

Only one thing jumped out during my first pass: the custom "Sticky" and "Password" fields need a little more work in order to function like their default counterparts. As they are, their visibility doesn't toggle - and, more importantly, they don't seem to save. Prefixing the ids of their respective span wrappers with custom- and updating their references in the JS to match should give us the expected toggle behavior (and help preserve the uniqueness of the ids from the default markup). As for saving, I wonder if the custom input values couldn't be assigned straight to the default hidden inputs? That could also potentially eliminate the need to provide custom hidden inputs. Last, I think there's a typo on https://github.com/washingtonstateuniversity/WSUWP-Content-Visibility/blob/fa67475a93dc4e5322f8405591b4bb7c47db70c0/js/post-admin.js#L28 (I suspect the selector should be #sticky).

jeremyfelt commented 8 years ago

Good catch. :)

After 0b572eb, sticky and password protected should be working as normal.

jeremyfelt commented 8 years ago

Checklist for release, hopefully tomorrow:

jeremyfelt commented 8 years ago

Docs written, blog post in draft, other things merged. Let's do it. :)