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

Add Functionality: Show next upcoming episode #309

Merged
merged 8 commits into from
Dec 1, 2020
Merged

Add Functionality: Show next upcoming episode #309

merged 8 commits into from
Dec 1, 2020

Conversation

chrisreddington
Copy link
Contributor

@chrisreddington chrisreddington commented Dec 1, 2020

There is an opportunity to "half" the screen real estate of the Most Recent episode section, and also use it as an opportunity to showcase the next episode that is due to be released.

This PR implements that with some logic to check if -

  1. Does enable_show_next_episode exist in the config?
  2. Is enable_show_next_episode set to true in the config?
  3. Are there upcoming episodes to show?

If all 3 conditions are matched, then the row partial will split the top section into two, allowing to show the next upcoming episode.

Fixes #301

@chrisreddington
Copy link
Contributor Author

@mattstratton - Right now, this doesn't implement any changes to the grid view. I wasn't sure how best to handle this scenario for the grid view, as there's no real concept of "latest" there...

One way could be to adjust the query in the grid view to also show the next upcoming episode, but it might not be as clear as it is in the row view. So, i'm curious on your thoughts on this one? Thx!

@chrisreddington chrisreddington marked this pull request as draft December 1, 2020 11:29
@mattstratton
Copy link
Owner

Right now, this doesn't implement any changes to the grid view. I wasn't sure how best to handle this scenario for the grid view, as there's no real concept of "latest" there...

Yeah, I suspect this feature would only work for the row layout anyway :)

REFERENCE.md Outdated
@@ -44,6 +44,7 @@ These should be set under the `[params]` section:
| `site_theme` | No | The color scheme for the overall site. Currently the options are `orange` (default), `grey`, and `blue`. | "blue" |
| `site_layout` | No | The layout for the home page. The options are `row` (default) or `grid`. | "grid" |
| `enable_jumbo` | No | When using the `row` layout, will set a jumbotron at the top instead of the sidebar. | "true" |
| `enable_show_next_episode` | No | When using the `row` layout, if there are episodes with the upcoming frontmatter set to true, then it will display the next episode on homepage. This halves the screen real estate of the latest episode section, which will then be displayed side by side. | "true" |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could give this a more descriptive name, like show_next_upcoming which makes it more clear that it is only for upcoming?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I'll make that change

@mattstratton
Copy link
Owner

I took a screenshot of what it looks like when enabled (for reference)

image

I think that we would want to do some styling on the "upcoming" episode; maybe draw a box around it with some other shading/bg color just to set it off?

Another option is to have less info for the "next episode" and make it a row by itself, but not include any images, etc, but just like a row across that says like "Next episode! Awesome Tech with George Bluth on 7 Dec 2020!" or something? If we did some kind of hero type row like that (narrow in my mind) it would work on grid too.

@mattstratton
Copy link
Owner

I am imagining something like this (rough mockup)
upcoming

@mattstratton
Copy link
Owner

mattstratton commented Dec 1, 2020

btw, if you sign off your commits, the DCO bot will stop flagging your PRs/commits as failed :) (no need to sign off on the existing ones, just saying for the future)

https://github.com/mattstratton/castanet/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco

GitHub
A podcast-oriented theme for Hugo. Contribute to mattstratton/castanet development by creating an account on GitHub.

@chrisreddington
Copy link
Contributor Author

@mattstratton - So this would work? :) (The name of the episode is a link, of course)

image

@mattstratton
Copy link
Owner

yes! that looks good. And should be a way to do it in grid view now too!

@mattstratton
Copy link
Owner

You might want to rebase this branch since I just merged a couple things, one of which touches REFERENCE.MD. It's probably a clean merge anyway but if you feel like rebasing it wouldn't hurt.

@chrisreddington
Copy link
Contributor Author

@mattstratton - Rebased, and committed. Also signed off the commits (thanks for the tip!)

Believe this is now ready for review.

@mattstratton
Copy link
Owner

Aha! it's a little trickier than that to edit the CSS files; you have to actually edit the SCSS files and re-compile them with gulp. There's instructions in the CONTRIBUTING.md file.

Long story short, yes, editing them by hand will work, but the next time they are compiled your changes will get blown away.

the good news is you really only need to edit the custom.scss file and as long as you use variable names, it'll update for the various color themes.

@mattstratton
Copy link
Owner

If the SCSS stuff is too tricky, let me know and I think I can checkout this branch/PR and make the changes for you!

@mattstratton
Copy link
Owner

Since the colors are the same for all the themes, I'm just going to do the gulp build for you and I'll push up the changes.

Taking me longer than I thought since the version of node-sass I've been using doesn't work on Big Sur :)

@chrisreddington
Copy link
Contributor Author

Hadn't spotted the scss in there, my bad - but good to know for future contributions :)

@mattstratton
Copy link
Owner

ugh. need to do some updates for node, etc...

@mattstratton mattstratton marked this pull request as ready for review December 1, 2020 22:23
@mattstratton
Copy link
Owner

okay, I think I got the build happy now! This looks clean and I'm going to merge it and cut a release.

@mattstratton mattstratton merged commit 928e71d into mattstratton:master Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] - Show upcoming episode on homepage
2 participants