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

Allow the hero to be filtered #724

Merged
merged 6 commits into from
May 27, 2015
Merged

Allow the hero to be filtered #724

merged 6 commits into from
May 27, 2015

Conversation

wjhdev
Copy link

@wjhdev wjhdev commented May 20, 2015

These Changes abstracts the featured media stuff that Ryan built to make it possible to filter the generated hero DOM for overriding in child themes.

Changes:

  • partials/single-hero.php is removed and replaced with two functions: largo_hero() (prints) and largo_get_hero() (returns).
  • Removes error checking where error checking is already done.
  • Replaces largo_hero_with_caption() with largo_featured_image_hero(). This is to keep consistency with newly introduced largo_featured_gallery_hero(), largo_featured_embed_hero(), &c. naming convention. This group of functions generate slightly different DOM for their respective hero type. Suggestions on how to handle depreciation appreciated. The function still exists to keep backwards compatibility, but we should have a system for depreciation (like WordPress).
  • adds largo_get_hero filter for overriding what is displayed by child themes.

aschweigert and others added 3 commits May 18, 2015 12:13
These changes abstracts the featured media stuff that Ryan built to
make it possible to filter the generated hero DOM for overriding in
child themes.

#### Changes:

 - `partials/single-hero.php` is removed and replaced with two
functions: `largo_hero()` (prints) and `largo_get_hero()` (returns).
 - Removes error checking where error checking is already done.
 - Replaces `largo_hero_with_caption()` with
`largo_featured_image_hero()`. This is to keep consistency with newly
introduced `largo_featured_gallery_hero()`,
`largo_featured_embed_hero()`, &c. naming convention. This group of
functions generate slightly different DOM for their respective hero
type. Suggestions on how to handle depreciation appreciated. The
function still exists to keep backwards compatibility, but we should
have a system for depreciation (like
[WordPress](https://developer.wordpress.org/reference/functions/_depreca
ted_function/)).
 - adds `largo_get_hero` filter for overriding what is displayed by
child themes.
@wjhdev
Copy link
Author

wjhdev commented May 20, 2015

This branch started from master with develop merged in, so Adam's 4311e9d commit made it in here.

@wjhdev
Copy link
Author

wjhdev commented May 21, 2015

Tests appear to be working now. The change in 5ab45b2 is to the actual test itself, which appears to have an erroneous assertEqual that was unreachable before.

I scratched my head a lot looking at these tests. I can't find anything documenting what WPAjaxDieContinueException is or why it gets fired. @rnagle, it might be a good idea to check that what I did is correct there.

@rnagle
Copy link

rnagle commented May 27, 2015

Thanks @willhaynes. I went ahead stubbed out tests for all of the new functions you added and factored the markup generated by largo_get_featured_embed_hero, largo_get_featured_image_hero and largo_get_featured_gallery_hero out into template partials for added flexibility. These changes have no practical effect on what you've done here, just another layer of flexibility and organization. Thanks for digging into this.

rnagle added a commit that referenced this pull request May 27, 2015
Allow the hero to be filtered
@rnagle rnagle merged commit dff73ed into develop May 27, 2015
@rnagle rnagle deleted the abstract-hero branch July 13, 2015 15:49
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