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

Continued byline changes #1265

Merged
merged 7 commits into from
Aug 4, 2016
Merged

Continued byline changes #1265

merged 7 commits into from
Aug 4, 2016

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Aug 3, 2016

Changes

  • Creates new conditional function largo_post_was_updated( $post ) in inc/conditionals.php
  • Uses largo_post_was_updated in Largo_Byline and its inheritor classes to determine whether to show the published date or the updated date
  • Updated date is only show if the post was updated and if the byline is being displayed in the context is_single()
  • The updated date now uses a <time> element, to match the published date
  • Removes the h5.top-time time element from the single post template, and styles for it
  • Removes styles hiding the date in the single-post and single-page bylines

Why

For #1259, continuing #1126.

For the Rivard Report.

@aschweigert
Copy link

so is the behavior here that we show either the updated date OR the published date but not both? are we still doing the XX hours ago for posts published/updated in the last 24 hours? or just posts published in the past 24 hours?

@benlk
Copy link
Collaborator Author

benlk commented Aug 3, 2016

Here's what shows:

published within 24 h published more than 24h ago
Updated Shows updated date Shows updated date
Not updated Shows the largo_time "XX hour ago" format Shows the published date

@benlk
Copy link
Collaborator Author

benlk commented Aug 3, 2016

let’s make the updated date also do the XX hrs ago thing
and then that’s fine to merge i think
some sites may want to show published and updated
but this should be the default

  • updated date in the XX Hours Ago format, as new function largo_time_updated

…y 'largo_modified_time' to the byline classes
@aschweigert
Copy link

lgtm, passing this to @rclations for a second set of eyes and then let's merge

@rclations
Copy link

everything looks good to me!

@benlk
Copy link
Collaborator Author

benlk commented Aug 4, 2016

And the failing tests are the usual, so I'll merge this and deploy it to Rivard.

@benlk benlk merged commit df78b32 into develop Aug 4, 2016
benlk added a commit to INN/umbrella-rivard-report that referenced this pull request Aug 4, 2016
@aschweigert aschweigert deleted the 1259-continued-byline-changes branch September 21, 2016 01:01
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

Successfully merging this pull request may close these issues.

3 participants