Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break up largo_byline to make it more modular. #1126

Closed
benlk opened this issue Feb 8, 2016 · 2 comments
Closed

Break up largo_byline to make it more modular. #1126

benlk opened this issue Feb 8, 2016 · 2 comments
Assignees

Comments

@benlk
Copy link
Collaborator

benlk commented Feb 8, 2016

Right now, it's a huge function: https://github.com/INN/Largo/blob/master/inc/post-tags.php#L84-L169

Make it a wrapper function around an action, maybe? The action would need to pass as arguments everything passed to largo_byline.

function largo_byline( $echo = true, $exclude_date = false, $post = null  ) {
    ...
    ob_start();
    do_action( 'largo_byline_action', array($echo, $exclude_date, $post) );
    // and on that is hooked with `add_action('largo_byline_action', name, priority, 3);` codex.wordpress.org/Function_Reference/do_action
    // - the author: https://github.com/INN/Largo/blob/master/inc/post-tags.php#L105-L142
    // - a separator
    // - the date: https://github.com/INN/Largo/blob/master/inc/post-tags.php#L146
    $output = ob_get_clean();

    $output = apply_filters( 'largo_byline', $output );

    // - the edit link and the separator before it: https://github.com/INN/Largo/blob/master/inc/post-tags.php#L159-L161
    // - echo vs return: https://github.com/INN/Largo/blob/master/inc/post-tags.php#L163-L167
}

The priorities of the actions on largo_byline_action should be separated by 10, to give us room to add new child theme actions in the middle, such as the twitter icon used in many child themes.

@benlk
Copy link
Collaborator Author

benlk commented Jul 8, 2016

In the current version of the branch with this work, c98d8ac, inc/post-tags.php is 1003 lines long, with the following breakup, by my rough count:

  • 290 lines for byline-related functions: largo_time, largo_author, largo_author_link, largo_byline, largo_byline_component_authors, largo_bline_component_publish_datetime, largo_byline_component_edit_link, largo_byline_component_sep, largo_byline_coauthors, largo_byline_normal_or_custom,
  • 260 lines for post social functions: largo_post_social_links, largo_floating_social_buttons, largo_floating_social_button_width_json, largo_floating_social_button_js
  • 160 lines for pagination functions: largo_entry_content, largo_custom_wp_link_pages, largo_content_nav,
  • 100 lines for excerpts: largo_excerpt, largo_trim_sentences,
  • 60 lines for comments: largo_comment,
  • 30 lines for post type icons: post_type_icon,
  • 50 lines for hero images: largo_hero_class, largo_hero_with_caption,
  • 20 lines for HTML metadata: largo_post_metadata

Can we break this up somehow?

  • The hero images stuff needs to move into inc/featured-media.php at least
  • Social stuff could probably go in its own file, with largo_social_links from inc/helpers.php
  • largo_post_metadata should go wherever largo_seo stays (currently inc/helpers.php)
  • Byline stuff might have a case for breaking off into its own file for byline stuff

Mostly, this file is getting unorganized and unwieldy.

@benlk
Copy link
Collaborator Author

benlk commented Jul 8, 2016

Also:

  • add twitter handle to normal users
  • add twitter handle to coauthors
  • add avatar to normal users
  • add avatar to coauthors
  • make the author things for normal users easier to modify
  • make the author things for coauthors easier to modify
  • css for authors avatars
  • css for author twitter handles; see RNS theme
  • test that the default = '' works for authors without set avatars.
  • stub tests and create ticket for creating tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants