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

#73 Enable summary for the post in blog #75

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

aldrineeinsteen
Copy link

@aldrineeinsteen aldrineeinsteen commented Jul 13, 2020

The changes add a clean navigation bar in the bottom of the blog.
image

The active element will be bold and highlighted

 - enabling summary params variable
@aldrineeinsteen
Copy link
Author

Enabled the configuration in params section for enabling blog summary
image

@aldrineeinsteen aldrineeinsteen changed the title #73 Enable summary for the post in blog Closes #73 Enable summary for the post in blog Jul 13, 2020
@aldrineeinsteen aldrineeinsteen changed the title Closes #73 Enable summary for the post in blog #73 Enable summary for the post in blog Jul 13, 2020
@aldrineeinsteen
Copy link
Author

Closes #73

@kentnek
Copy link
Collaborator

kentnek commented Jul 13, 2020

Thank you for your contribution!

The changes add a clean navigation bar in the bottom of the blog.

Could you limit the scope of this PR to only adding the summary? Also a screenshot of the summary in the list would be great.

Regarding your bottom bar, could you separate it to a different PR, and elaborate on the use case?

@jakewies
Copy link
Owner

@aldrineeinsteen thanks for this work! I agree with @kentnek, let's try and keep this one to the addition of an optional summary in the post list only.

Also, I notice git picking up some changes related to formatting. This shouldn't be the case because we have prettier running on any files staged for commit. When pulling the repository did you run yarn install? Check out the CONTRIBUTING.md file for more info, and ping here if you need any help 😄

@aldrineeinsteen
Copy link
Author

Thank you @jakewies / @kentnek ; Let me segregate the PR into two

  1. Summary info - options
  2. Pagination

exampleSite/config.toml Outdated Show resolved Hide resolved
@kentnek
Copy link
Collaborator

kentnek commented Jul 14, 2020

Could you replace the screenshot in the PR to that of the "summary"?

layouts/_default/list.html Outdated Show resolved Hide resolved
- updating the configuration from params.blog to params
- documentation has been added to enable configuration of summary for blogs

Closes jakewies#73
@kentnek
Copy link
Collaborator

kentnek commented Jul 14, 2020

I think the code looks good now, but the styling needs some magic from @jakewies.

In my opinion, the summary text and "Read more" should be slightly lighter than the main title. Also I'd prefer the "Read more" button to be left-aligned so it doesn't break the vertical flow of the page.

@jakewies
Copy link
Owner

@kentnek as far as formatting goes, I made a mention of this in an early review. The comment is still unresolved. I'm waiting to here back from @aldrineeinsteen on the topic.

I'll take a look at the styles later today and work my magic.

@aldrineeinsteen
Copy link
Author

hat of the "summary"?

done :)

@jakewies
Copy link
Owner

Finally got around to making style adjustments on this PR.

Capture

A few notes here:

  • We will need to document how Hugo renders these summaries. You can find out more here. You'll notice in the picture above that the second post has some broken formatting. That's because it does not use Hugo's summary declaration. The first post does. My thoughts are that we update the README to include a link to this documentation and suggest that if users want to add a summary, they do so using the documented syntax.
  • There seems to be some conflicts with regenerated assets. I'm assuming the best way to resolve would be to rebase against the master branch. I'll handle that.

@kentnek let me know your thoughts on the visuals. If things are good to go I'll handle the conflicts and we can merge this one.

@kentnek
Copy link
Collaborator

kentnek commented Jul 29, 2020

You'll notice in the picture above that the second post has some broken formatting.

I think it's a good idea to add the <--more--> summary divider to that example post, so it'd render better and also showcase to the user they can do that too.

@jakewies
Copy link
Owner

I thought the same. I didn't know if it was frowned upon to update the default example content provided by Hugo.

But I'll go ahead and clean those up.

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