CUNY Academic Commons

And here's a reply from Heather Sutton:

Thank you, Marilyn and Boone! The GC emails for our team who will be admins on the site are jkelly@gc.cuny.edu and hsutton@gc.cuny.edu

Best,
Heather

Thanks, Alan. I'm happy to provide recommendations on WordPress best practices or answers to any questions you might have.

I've had a chance to look through the zip file that you sent and I have a bit of feedback.

First, a general note about our policies and procedures for custom themes. You can read more details at https://dev.commons.gc.cuny.edu/hosting-partner-handbook/, but an important point for this project is that all custom code has to come through me and the development team, and it will be released to the production website only after review, and only during regularly scheduled bi-monthly maintenance releases.

This makes it difficult to do quick iterations on your work. So, for example, it appears that you've hardcoded lots of content into templates, especially in the case of page.php. It's OK with me if you do this, but note that it will therefore not be editable by the site admin, and will only be editable through the procedures for updating custom code. This kind of turnaround might not be ideal for your team. In contrast, if this content were stored inside of WordPress's posts, your PHP template could load the content from the database and display it (the_content();) while remaining easily editable by members of the team.

Similarly, I see that some parts of the theme seem to be unfinished. For example, there's a section on index.php that contains some placeholder text ("Insert some text here about the subject"). If we release this as-is, there will be no way to fix it until the following maintenance release. So we need to be sure that the theme is ready to go before putting it into action, because quick edits will not be possible.

In addition to this general comment, there are a few conventions of WordPress development that need to be adhered to, or the theme won't work in the Commons environment:
- Perhaps most importantly, you must be sure that URL paths are built dynamically. In some places you have hardcoded paths like http://cunygraduatelmis.local, which obviously won't work in production. There are lots of tools in WordPress to build dynamic links, but for your purpose I recommend the following: Anywhere you need to reference a path that's inside the theme directory, use get_stylesheet_directory_uri(), and anytime you need to reference a path relative to the site URL, use home_url(). Examples:

<img class="logo" src="<?php echo esc_attr( get_stylesheet_directory_uri() ); ?>/images/thumbnail.png"> // instead of hardcoded local path

<a href="<?php echo esc_attr( home_url( 'resources' ) );"> RESOURCES </a> // instead of "/resources" 

- It's important to load the local version of jQuery instead of one from a CDN, or it'll cause conflicts with the rest of the Commons JavaScript. This means enqueuing in functions.php rather than a hardcoded <link>, and probably also means that you need to enqueue other scripts as well, to ensure the proper dependency relationships. See https://developer.wordpress.org/themes/basics/including-css-javascript/

As for the rest of the migration, it looks like you have a very small number of pages, so it's not really necessary to do a full SQL import. When the time comes, I recommend that you use the built-in WP export/import tools to move your content over. See https://help.commons.gc.cuny.edu/importing-and-exporting-site-content/. The main part of the migration will be the theme handoff, so in the future it's OK to give me a zip only of the wp-content/themes/cuny-graduate-lmis directory.

Hi Mr. Gorges,

Thank you so much for the in-depth code review and the helpful links to help me understand this process. I understand what I have to change, and I will try to implement the changes you suggested in this post. I will try to have it done within the next few days, since I am busy with schoolwork at the moment (finals and projects). Will it be fine to resend the updated version for you to check whenever these changes are fixed? This version will not be the complete one, but hopefully, it will be able to load links and content dynamically -- I was working on this assignment during the summer, and I agreed to work on it intermittently during the semester. However, I got bombarded with a lot of work, so I could not get to it, and LMIS wants to have their website done by the end of December ideally. The template that I will be sending in the end will not be finalized, but it should be good enough to load all the current content for the next person to work on it.

Best,
Alan Li

Thanks, Alan. There's no rush from my end. Please just upload a zip file to this ticket (or provide a link to a public repository) so that I can review the theme changes.

Hi Mr. Gorges,

I hope you are having a good day today. I just finished my finals for this semester, so I will be finalizing the changes to my code; I am so sorry for the huge delay. I intend to finish working on the code tonight or by tomorrow. Is it still fine to send you the code to review? If so, when are you available in the next week or so since the holidays are coming up? I am available for the entirety of December, so I can respond as soon as possible if there are any issues regarding the code.

Best,
Alan Li

Hi Alan - I'll be available to review early next week if you can have something to me this weekend. We have a regularly scheduled release on Dec 22 and it would be nice to get this in by then, if we can complete our back-and-forth by that point.

Thanks for the update, Alan.

As an example, in the Impact page (Partners->Impact) (ignore the alignment), would I create a post for each card? How would I be able to create divs with particular classes to create this effect? Additionally, there's an anchor tag on this page that opens a pdf in another window; how can this be done by writing all the content in the page itself? I have been looking up and trying to solve this question for the last two days. I understand that it is not necessary to do this implementation using the_content(), but I still want to try it because in addition to making it easier to editors, I also want to comprehend the structure/format that developers use for the sake of learning.

There are various ways to accomplish this. One is through the use of custom blocks. See eg https://deliciousbrains.com/custom-gutenberg-block/. But this may be a lot of work if you don't have a background in the necessary technologies. Another way is by leveraging WordPress shortcodes, which can be a fairly user-friendly way of creating specific kinds of markup. See eg https://developer.wordpress.org/plugins/shortcodes/ You could also try simply building the content in the WordPress editor as close as reasonably possible, and then using the default classes (along with perhaps some of the specific body classes that WP adds to its markup, to differentiate between pages) to target elements, rather than using custom classes in the markup.

If none of these routes seems viable, it's probably fine to stick with page templates. But they should be defined and loaded in the correct way. Page Templates can be associated with specific pages if they are defined properly; see https://developer.wordpress.org/themes/template-files-section/page-template-files/. Alternatively, you can specify a template file to be used with a specific page using the naming conventions in the WP Template Hierarchy. See https://developer.wordpress.org/themes/basics/template-hierarchy/

I changed all of the hardcoded paths to the method you provided in your response; is this fine now?

This looks good in some places, but searching the theme for '.local' I still see many instances.

I also took out the jQuery CDN, and I did wp_enqueue_script('jquery') in the functions.php instead. Afterwards, I loaded up my own script in the same function. Will this cause an issue in the future where the jQuery will somehow load slower than my personal jQuery script? Additionally, you mentioned specifically to change the jQuery CDN, so I did not change the Bootstrap one at all. Just to double-check, should I also place the Bootstrap script and link inside functions.php?

I still see that jQuery is being loaded via script tag at the top of index.php. Using the CAC version is the best way to ensure compatibility going forward, and it should not cause performance issues. It is OK to load Bootstrap from the CDN as you've done.

As an aside, the outcome of the code review is to be able to "migrate" the current progress into CUNY Academic Commons; I will not be finishing up the project, so it has to be usable by the next person. Sorry for the lengthy response, and I am super grateful for your help. I came into the project with no knowledge in PHP at all, so I apologize once again for my lack of understanding and messy code.

Understood. No apologies are necessary - the requirements of the Commons means that the process is more complex than it might otherwise be. Thanks for your patience working through these issues.

Thank you for the prompt response Mr. Gorges,

I will look into custom blocks and shortcodes to see if I can utilize either of them to implement the changes necessary. I am not 100% confident that I can do it, but I will attempt it, since it will be easier for others in the future.

Regarding the routes, I can change the ones with no route to have a page template; this seems simple and practical to accomplish. I will take a look at the documentation to comprehend the material better as well.

A lot of the .local that I see are commented out aside from the one in the home page, which I will convert now. I can get rid of all instances of '.local' indefinitely by the next iteration of the code.

I forgot to remove the script tag for jQuery at the top of index.php; if I take it out and just use the one in functions.php, does this mean I am using the CAC version?

I sincerely thank you for being so patient and thorough with the code review -- I have learned a lot during this migration process. I will be attempting to use custom blocks or shortcodes today so that I can have another updated version by tomorrow. I understand that you might be busy tomorrow, so let me know when and if it is possible for you to look over the hopefully final iteration of the code in the future.

Best,
Alan Li