INN / umbrella-inndev

Umbrella repository for inn.org
GNU General Public License v2.0
0 stars 1 forks source link

Build template for presenting content on amplify.inn.org landing page #124

Closed benlk closed 4 years ago

benlk commented 4 years ago

Wireframe: https://innorg.slack.com/files/UGQMT8CEB/FR0APR126/amplify.pdf

benlk commented 4 years ago

Wireframe: condensed

benlk commented 4 years ago
joshdarby commented 4 years ago

Findings so far:

benlk commented 4 years ago

Adding a shortcode to the Link Roundups plugin to output a list of Saved Links seems like the easiest option to maintain. It would also make a nice benefit for all other users of the Saved Links plugin.

But the effort of building and testing a modification to that plugin, and releasing the updated plugin, seems a little beyond this scope of this project, unless @kaylima or @MirandaEcho approve.

joshdarby commented 4 years ago

FWIW, the display-posts plugin shortcode works great on the frontend for displaying saved links, but causes Link Roundups to throw a few 500 errors when trying to edit the post

Deprecated: Methods with the same name as their class will not be constructors in a future version
of PHP; saved_links_widget has a deprecated constructor in 
/Users/josh/Desktop/Work/umbrella/wp-content/plugins/link-roundups/inc/saved-links/class-saved-links-widget.php on line 7

Fatal error: Uncaught ArgumentCountError: Too few arguments to function
SavedLinks::get_excerpt(), 0 passed in /Users/josh/Desktop/Work/umbrella/wp-content/plugins/link-roundups/inc/saved-links/class-saved-links.php
on line 549 and exactly 1 expected in 
/Users/josh/Desktop/Work/umbrella/wp-content/plugins/link-roundups/inc/saved-links/class-saved-links.php:673 Stack trace: #0 /Users/josh/Desktop/Work/umbrella/wp-content/plugins/link-roundups/inc/saved-links/class-saved-links.php(549): 
SavedLinks::get_excerpt() #1 /Users/josh/Desktop/Work/umbrella/wp-includes/class-wp-hook.php(288): SavedLinks::the_excerpt('<p>test!&ndash;...') 
#2 /Users/josh/Desktop/Work/umbrella/wp-includes/plugin.php(206): WP_Hook->apply_filters('<p>test!&ndash;...', Array) 
#3 /Users/josh/Desktop/Work/umbrella/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php(1561): apply_filters('the_excerpt', 'test!&ndash;hi') 
#4 /Users/josh/Desktop/Work/umbrella/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php(472): WP_REST_Posts_Cont in /Users/josh/Desktop/Work/umbrella/wp-content/plugins/link-roundups/inc/saved-links/class-saved-links.php on line 673
benlk commented 4 years ago

Does that same error happen when the display-posts plugin is deactivated?

joshdarby commented 4 years ago

Nope.

joshdarby commented 4 years ago

Updating the get_excerpt function to not require an arg fixes the issue on the editor, though

https://github.com/INN/link-roundups/blob/be0f2a93a892904bf1d55a9dfd8ccf9311822d87/inc/saved-links/class-saved-links.php#L760-L776

public static function get_excerpt( $post ) {
    $post   = get_post( $post );
    $custom = get_post_meta( $post->ID );
    ob_start();
    if ( isset( $custom['lr_desc'][0] ) ) {
        echo '<p class="description">' . $custom['lr_desc'][0] . '</p>';
    }
    if ( isset( $custom['lr_source'][0] ) && ( $custom['lr_source'][0] != '' ) ) {
        echo '<p class="source">' . __( 'Source: ', 'link-roundups' ) . '<span>';
        echo isset( $custom['lr_url'][0] ) ? '<a href="' . $custom['lr_url'][0] . '">' . $custom['lr_source'][0] . '</a>' : $custom['lr_source'][0];
        echo '</span></p>';
    }
    $html = ob_get_clean();
    return $html;
}

to

public static function get_excerpt( $post = null ) {
    if( $post ){
        $post = get_post( $post );
        $custom = get_post_meta( $post->ID );

        ob_start();
        if ( isset( $custom['lr_desc'][0] ) )
            echo '<p class="description">' . $custom['lr_desc'][0] . '</p>';
        if ( isset( $custom['lr_source'][0] ) && ( $custom['lr_source'][0] != '' ) ) {
            echo '<p class="source">' . __('Source: ', 'link-roundups') . '<span>';
            echo isset( $custom['lr_url'][0] ) ? '<a href="' . $custom["lr_url"][0] . '">' . $custom["lr_source"][0] . '</a>' : $custom["lr_source"][0];
            echo '</span></p>';
        }
        $html = ob_get_clean();

        return $html;
    }
}

But again, that would require a new plugin release :(

joshdarby commented 4 years ago

And no, including the include_excerpt="true" arg on the shortcode doesn't fix it

kaylima commented 4 years ago

Adding a shortcode to the Link Roundups plugin to output a list of Saved Links seems like the easiest option to maintain. It would also make a nice benefit for all other users of the Saved Links plugin.

But the effort of building and testing a modification to that plugin, and releasing the updated plugin, seems a little beyond this scope of this project, unless @kaylima or @MirandaEcho approve.

@benlk what's the estimate for the modification and release?

joshdarby commented 4 years ago

what I need to know is:

  • we researched options and this is what we recommend (I don’t need to know about the options > that won’t fit/work)
  • here’s how much time it will take
  • any known risks, dependencies or other considerations

@kaylima I think updating Link Roundups is the option that we'd recommended, especially since it'll probably help us down the road and offer another feature for users that they might be wanting anyways.

But, in terms of scope, since we would need to:

I'd say a fair estimate would be over 2 hours but under 4. Does that sound fair, @benlk?

In terms of risks, I don't think there would be any, except we'd need to make sure we let everyone know that we've released a new feature to the plugin.

benlk commented 4 years ago

I'd estimate that current release checklist for Link Roundups runs 2-4 hours by itself: https://github.com/INN/link-roundups/issues/184

The changes described in this ticket would be less than 2h, including documentation. Another 2h if we added a server-side-rendering block to wrap the shortcode, like how INN/pym-shortcode does it.

There are a number of known minor bugs in the Link Roundups plugin that might be nice to address: https://github.com/INN/link-roundups/issues?q=is%3Aopen+is%3Aissue+label%3A%22type%3A+bug%22

benlk commented 4 years ago

If the ~8 hours above is too much, and we're really looking to save on time, we could instead just embed the embed seen on member sites at full width here on inn.org. The "Load More" button would be part of the widget, rather than a separate thing here.