Project

General

Profile

Feature #3330

Feature #2945: My Commons

"Commons Information" tool

Added by Boone Gorges about 5 years ago. Updated almost 5 years ago.

Status:
Assigned
Priority name:
Normal
Assignee:
Category name:
My Commons
Target version:
Start date:
2014-07-16
Due date:
% Done:

0%

Estimated time:

Description

See #2945 and http://redmine.gc.cuny.edu/attachments/download/1293/Commons-PersonalHomepage-Options-v3.pdf

We need a plugin that will power the Commons Information section of the profile and personal home page. Based on the wireframes and team discussions, I'm going to suggest the following for the first iteration (keeping in mind that we can add fancy stuff in the future):

- New custom post type: "Commons Information". Available to Editors and Administrators (current_user_can( 'edit_others_posts' ))
- The Commons Information "widget" won't really be a widget, but just a function that spits out some markup
- Widget should just show the most recent item + featured image. Mockup doesn't show titles, but I imagine we'll want them - right below where the image appears
- The 'show more' link at the bottom will fade the content of the widget, send an AJAX request, and show the next item (and cycle back to the first one when done). Don't show the 'view more' link on noscript.

Chris, feel free to chime in with additional thoughts or suggestions.

Dan, are you up for the challenge? :) It's a bit tricky because you'll have to develop it in isolation (you won't be able to put it in its proper place in the theme yet because that place doesn't exist yet; see #3329), but I don't think that'll be a serious problem. Let me know what you think, or if I can help get you started somehow.


Related issues

Related to CUNY Academic Commons - Feature #3357: Commons Information Tool: Purpose and ProcessResolved2014-07-30

History

#1 Updated by Daniel Jones about 5 years ago

Yes I'd love to take a crack at this! I'll let you know when I come up against roadblocks or have other clarifying questions.

What's our standard on browser compatibility? For when I'm working on the fade-out and show the next item.

#2 Updated by Daniel Jones about 5 years ago

I've created a branch off of 1.7.x with the Commons Information plugin. Here's the change set with what I've added so far - https://github.com/cuny-academic-commons/cac/commit/c7ae2b3c16ba08472ba4c39cde04d9a144f1dcb9

It looks like the custom post type is working, but I think the naming could be better (gets especially awkward with the capabilities), and also I'm not very familiar with internationalization so a little guidance on that would be helpful. I ran into a little issue where when I tried to prefix the custom post type with 'cac' it became too long (can only be 20 characters). Should I leave it as is, or change it to something like 'cac_information'? The way I restricted the post type to admins and editors seemed like the best most flexible way to go - creating new capabilities and assigning them to roles - but if you think there's a better way let me know.

I've also started working on the function to output some markup for the homepage, and have something very basic working without the view more link yet. I haven't found a good way of testing it though - right now my very poor solution is to create a template for a single commons information post that calls my cac_commons_information() function. The problem there is I don't know what to add in terms of CSS because that template isn't pulling in anything from the theme. Any thoughts on the best way to fix this? Should I just add calls to get_header() and get_footer()? I tried messing around with the homeDev.php page template but no go.

Also - on the AJAX request. Would it be possible to fire the request for the second item on page load, store the information (title, link to the thumbnail, excerpt) in javascript variables, and then display the next item right away when view more is clicked as well as doing another AJAX request to pull in the information for the next item? I guess it wouldn't save that much time, but that way the database querying would only be happening while the user already had something to look at, and the transition to the next item could happen immediately without waiting for the request to go through. Does that make sense? Or is it not worth it?

Thanks for any help!

#3 Updated by Boone Gorges about 5 years ago

What's our standard on browser compatibility? For when I'm working on the fade-out and show the next item.

We try to support IE8 at least minimally, so let's try to do any fancy JS or CSS in a way that degrades gracefully. We use jQuery extensively, so feel free to use it for this purpose. If I were building this, I'd use CSS3 animations and simple $.addClass() and $.removeClass(), but feel free to use $.fadeIn() or whatever if that's easier.

I'm not very familiar with internationalization

Brief version: do this with all non-echoed strings:

'name' => __( 'Commons Information', 'cac-commons-information ),

I ran into a little issue where when I tried to prefix the custom post type with 'cac' it became too long

Yeah, this is a crummy quirk in the API. It's probably more important to maintain the cac_ prefix, to guarantee uniqueness. Go with 'cac_commons_info'.

The way I restricted the post type to admins and editors seemed like the best most flexible way to go - creating new capabilities and assigning them to roles - but if you think there's a better way let me know.

Yeah, this is great - you've gone above and beyond. (I probably would've left the defaults, which would piggyback on 'edit_others_posts', etc.) One thing: calling $role_object->add_cap( $cap ); on 'init' is not a great idea, because it requires a database UPDATE on each pageload. Move your role-saving logic into a function that only gets called during activation, instead of doing it inside the function where you register the post type.

The problem there is I don't know what to add in terms of CSS because that template isn't pulling in anything from the theme. Any thoughts on the best way to fix this? Should I just add calls to get_header() and get_footer()?

Yeah, sorry about this - like I said above, this work needs to wait for some other templating work before it can be really finished. If I were you, for testing I would just hardcode it directly into our home template, maybe in one of the widget columns in the second section (Groups, Members etc). That way you'll be dealing with something roughly widget-width.

n the AJAX request. Would it be possible to fire the request for the second item on page load, store the information (title, link to the thumbnail, excerpt) in javascript variables, and then display the next item right away when view more is clicked as well as doing another AJAX request to pull in the information for the next item?

Definitely - this is a great idea. The only stipulation is that I'd prefer if you didn't load all available items on the initial pageload. Beyond that: you can load the 2nd item immediately, and load the 3rd when you display the 2nd; or you can load all of them in one or more AJAX requests that get fired after the page is rendered. Whichever way seems most elegant to you.

#4 Updated by Daniel Jones about 5 years ago

Have a version of the plugin working locally - let me know if you can get it to work. Change set is at https://github.com/cuny-academic-commons/cac/commit/f29cdc697977c16e1a091b2b4f8bcb6e07d1ef2f

I integrated the it with the home page sidebar. After our dev team check-in today I added in links for both previous and next items (previous meaning newer, next meaning older). I've left them as simple links for now and haven't added in any transitions until we have a better sense of what the UI should be like. Added AJAX calls to make sure that both the next and previous Commons Information posts are available before the user clicks the link - still need to add in the noscript tags to hide the links. I think I've done the right setup for internationalization, and I also took the code that updates the capabilities and put it into the register_activation_hook callback. Is there a reverse of register_activation_hook that I should set up to remove those capabilities when the plugin is deactivated? I also took out support for excerpts in the custom post type since it doesn't seem like we're giving the posts their own post templates for now - but let me know if I should change that.

I had a lot of fun teaching myself AJAX for jQuery and Wordpress, but definitely could have gone about it the wrong way. Let me know what you think. Thanks!

#5 Updated by Boone Gorges about 5 years ago

Hi Dan,

Really nice work! This is looking good so far. AJAX setup looks pretty much perfect, and your technique for loading the next item is clever. I have some comments, mostly minor. I also have a couple questions that I think need to be passed back to the team (in terms of design and desired functionality)

- 'Previous' and 'Next' buttons should be sensitive to where you're at in the order. If you're on the last item, don't show a Next button. Alternatively, have these buttons loop around.
- You're using '#' as the href for these buttons. This is fine, but be sure to prevent the default browser behavior when clicking, or you get jumped to the top. event.preventDefault() or even `return false;` are fine. An alternative strategy is to change the href attribute to an anchor that sits at the top of the Commons Information box; the advantage here is that if you have very long content in the box, you get jumped back to the top of it. But see below on long content.
- Be consistent about naming your PHP functions. Pick a prefix and use it throughout. 'cac_ci_' or something like that.
- Localization looks good. When/if packaged for distribution, I'll walk you through creating a POT file.
- Really long posts mixed with really short posts can mess up formatting when moving from slide to slide. You mentioned that you didn't use excerpts, but I think that excerpts are the solution. As a consequence (and as you note) that'll mean that there'll have to be separate landing pages for individual information objects. I think this is desirable anyway. I suggest the following changes for now: use get_the_excerpt() for display; add 'excerpt' support to register_post_type(); make the <h3> title element a link to the permalink; and just go with the default page template for the permalink for now. Then I think we'll pass this along to Chris for some feedback on how it should look.
- You can use register_deactivation_hook() to remove the custom caps.

Thanks!

#6 Updated by Daniel Jones about 5 years ago

Thanks Boone! I think I've made all the changes you laid out - https://github.com/cuny-academic-commons/cac/commit/3f7a6e7cdd13327e068142b11c36ba560c488ef3 and also https://github.com/cuny-academic-commons/cac/commit/cdff86e4496dc70009283b848dbfde2dcab07e8f

Ignore the backup files in the first change set- accidentally added them to repo but they're removed now I pushed another commit to get rid of them on github.

I decided to just go with e.preventDefault() for now for the links.

I had a UX question - right now the "next" link takes you to an older post, and the "previous" link takes you to a newer post. The fact that when you load the page it shows you just the "next" link might make it easier, but I imagine that could start to feel a little counter-intuitive. Even tripped me up as I was going back over the code. Should we just change it to "older" and "newer" or leave it or something else? I guess when it's 100% done there might not even be text so maybe we don't have to worry about it for now.

#7 Updated by Boone Gorges about 5 years ago

Thanks, Dan. Changes are looking great.

I had a UX question - right now the "next" link takes you to an older post, and the "previous" link takes you to a newer post. The fact that when you load the page it shows you just the "next" link might make it easier, but I imagine that could start to feel a little counter-intuitive. Even tripped me up as I was going back over the code. Should we just change it to "older" and "newer" or leave it or something else? I guess when it's 100% done there might not even be text so maybe we don't have to worry about it for now.

My suggestion is to use a combination of styles and arrows to make the point clearer:

// First item
--------------------------------------
 Title

 Text text text text text
 text text text text

                            Next ->
---------------------------------------

// Middle item
--------------------------------------
 Title

 Text text text text text
 text text text text

<- Previous          Next ->
---------------------------------------

// Last item
--------------------------------------
 Title

 Text text text text text
 text text text text

<- Previous
---------------------------------------

Then let's Chris make any further decisions about what needs to be done UX-wise.

Seeing this in action, I'm getting a bit more concerned about the shifting height of the text, depending on its length. I'd like to suggest the following changes toward this end:
- Before using get_the_excerpt() to output the text of the information, check to make sure it's below a certain length. (If no excerpt is manually created, the entire post text will be output.) If not, trim it down to a certain number of words/characters. You might want to use the BP function bp_create_excerpt(), which will do most of the necessary work of determining line endings, etc.
- Set some sort of static height on the element, so that clicking Previous and Next will not change the height of the widget (and ideally, so the Previous/Next buttons would not move at all)

Once that's in place, we'll pass it along to the rest of the team for some feedback on what's there so far, and whether we should move forward with more specific styling for the standalone pages.

#8 Updated by Daniel Jones about 5 years ago

Thanks again Boone - I made the changes you suggested at https://github.com/cuny-academic-commons/cac/commit/e1ab4328673b18b2656661e4eabb9061be29d15e. Made arrows for the buttons and positioned them absolutely relative to the Commons Information container. Also gave the container a static height. I used bp_create_excerpt() to truncate the text to 200 characters. The first commit I made I forgot to do it on the AJAX callback end but fixed that.

Last things was a javascript error that I hadn't caught before that wouldn't let you go back to the first announcement until you had gone to the third, but it's fixed now.

Let me know what the next steps are, and thanks for all of your help on this!

#9 Updated by Boone Gorges about 5 years ago

Thanks, Dan! A couple things:

1. I made a few styling tweaks to make sure that the Previous and Next buttons didn't get covered by the excerpt: https://github.com/cuny-academic-commons/cac/commit/08e409c19fa91169124f59d581378732f71071ca
2. I changed some syntax for better PHP version compatibility (it was throwing errors on cdev) https://github.com/cuny-academic-commons/cac/commit/584227c2d150c94981eaa5cb7d4e69c4f286f226
3. I've merged the works in with the 1.7.x branch. Let's continue work there.

Overall it seems to be working pretty well. I did notice some oddness when working on the rather slow cdev site - namely, clicking Previous and Next always resulted in AJAX calls, even to view items that had already been loaded. It's far more efficient to cache this stuff. Can I ask you to work on something along those lines? Roughly:

var cached_items = [];

// ...
function loadNextCommonsInformation() {
    // ...
    if ( typeof cached_items[announcementToLoad ] !== 'undefined' ) {
        nextAnnouncement = cached_items[announcementToLoad];
    } else {
        // do the AJAX request and then cache results
    }
}

I'm going to move discussion of UX stuff over to #3357. Thanks for your work on this!

#10 Updated by Daniel Jones almost 5 years ago

Thanks for fixing those problems, Boone! What version of PHP do you think should I develop for in the future? I know I've run into some problems with plugins because of changing PHP syntax myself and don't want that to come up for others.

I made the changes to the javascript so that it caches and re-uses the information from the posts - https://github.com/cuny-academic-commons/cac/commit/ea92abc901d963a61afbda05081abc175d8e18e4. Since I'm running locally it's hard for me to tell if it's actually stopping it from running all those unnecessary database queries but it looks like it ought to be. Can you let me know if it's working?

I can't think of any case in which a previous (newer) announcement wouldn't be cached when a user went back to it, but I figured better safe than sorry and kept the same controls for loadPreviousCommonsInformation() as loadNextCommonsInformation(). Let me know if you think I should change it and just leave it in for the loadNext function.

#11 Updated by Boone Gorges almost 5 years ago

What version of PHP do you think should I develop for in the future?

The Commons production server is currently running PHP 5.3.3. So you should build for that at a minimum. WordPress supports all the way down to 5.2.4, so if you are building something that you want to make available more broadly, you should keep that lower requirement in mind.

Since I'm running locally it's hard for me to tell if it's actually stopping it from running all those unnecessary database queries but it looks like it ought to be. Can you let me know if it's working?

Yup, this is perfect. Your best friend for debugging AJAX is your JS console. In Firebug, the regular Console view will show you the details of all AJAX requests; in Webkit browsers, go to Network, then hit the Filter icon, then choose the XHR filter.

kept the same controls for loadPreviousCommonsInformation() as loadNextCommonsInformation(). Let me know if you think I should change it and just leave it in for the loadNext function.

Yup, this looks fine. No harm in it.

Let's discontinue the commons-information branch at this point, and do all future work on this feature directly in the shared 1.7.x branch.

Thanks again for your work! Follow #3357 for more discussion on UX issues. We'll close this one when that ticket settles whether we're feature complete for now.
Can't

#12 Updated by Boone Gorges almost 5 years ago

  • Status changed from New to Assigned
  • Assignee changed from Daniel Jones to Chris Stein
  • Target version changed from 1.7 to Future release

As discussed in a previous phone call, this feature is unfortunately being pulled from the 1.7 milestone. Dan, you've done a great job with it, and I think we'll find a place for it in a future release.

Reassigning to Chris for consideration in the context of a redesign.

Also available in: Atom PDF