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

Implement largo_maybe_top_term #1319

Merged
merged 3 commits into from
Sep 30, 2016
Merged

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Sep 29, 2016

Changes

  • Adds function largo_maybe_top_term( $args ), which outputs largo_top_term( $args ) wrapped in an h5.top-tag if largo_top_term( $args ) would return a link. If no link is returned, then it does not output the h5.top-term. $args are the same for each function.
  • Adds a blank test for the function, so that we know what needs to be tested in it when we eventually write a test for it.

Why

@benlk benlk added type: bug type: improvement priority: normal Must be completed before release of this version of plugin. labels Sep 29, 2016
@benlk benlk added this to the 0.5.5 - Story Elements milestone Sep 29, 2016
@aschweigert
Copy link

I'm ok with this for now, although the new default will/should be to just show the top term instead of doing some of this crazy conditional stuff. That's all very legacy at this point (a holdover from when people had the option of EITHER the top term or showing a list of tags below the excerpt in various rivers of stories). At this point, I think it makes sense to just show the top term by default in any of these contexts.

@aschweigert aschweigert merged commit ea1c382 into develop Sep 30, 2016
@aschweigert aschweigert deleted the 1318-largo_maybe_top_term branch September 30, 2016 13:25
@benlk
Copy link
Collaborator Author

benlk commented Sep 30, 2016

This is mainly for the cases where no top term is set, which is still a case that we need to worry about.

benlk added a commit that referenced this pull request Oct 12, 2016
aschweigert added a commit that referenced this pull request Oct 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: normal Must be completed before release of this version of plugin. type: bug type: improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants