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

PR for #12 Highlight related articles at the bottom of each article. #25

Merged
merged 2 commits into from
Aug 10, 2021
Merged

PR for #12 Highlight related articles at the bottom of each article. #25

merged 2 commits into from
Aug 10, 2021

Conversation

MuluhGodson
Copy link
Contributor

From your previous comment, I optimized the code in a way that gives you the flexibility to determining what relates the articles. This is done in the front matter via the categories variable. articles in the same category will be related. Also if an image is specified in the Front matter of the article, this image will be used on the card when displaying the article as related to another, else it will just display a title and a description. I noticed each article now has a description (due to the OpenGraph PR).

In the config.toml under the general parameters, I added a new variable article_count which determines the maximum number of articles to be displayed in the related section. Currently, this value is set to 3 so at most 3 articles will be displayed. The current article being read is excluded from being displayed as related to itself.

@jwflory jwflory self-assigned this May 4, 2021
@jwflory jwflory added the T: new change Adds new capabilities or functionality label May 4, 2021
@jwflory
Copy link
Member

jwflory commented May 4, 2021

Hi @MuluhGodson, thanks for submitting this! The changes to the inventory repository make sense, but I noticed that you made new commits directly to the git submodule. This is a confusing step and I need to write better documentation.

First, I am curious if upstream would like to integrate these changes upstream. @somratpro, if @MuluhGodson sent this as a Pull Request to themefisher/dot-hugo-documentation-theme, would you be interested in accepting this contribution? This is the easiest option for me as a downstream maintainer.

However, if @somratpro prefers to not take these changes, we need to raise a Pull Request instead on the unicef/inventory-hugo-theme repository. We do not want to carry custom commits to the submodule in this repository. Committing it to the inventory-hugo-theme repository improves the likelihood that others will be able to use these changes in building other downstream works.

Once the upstream theme or the UNICEF downstream fork is updated with the changes, I will update the git submodule for this repository, and then this Pull Request will be ready for a git rebase.

@jwflory
Copy link
Member

jwflory commented May 7, 2021

Hi @MuluhGodson, feel free to send your theme changes to unicef/inventory-hugo-theme to move this Pull Request forward.

@MuluhGodson
Copy link
Contributor Author

MuluhGodson commented May 7, 2021 via email

@jwflory
Copy link
Member

jwflory commented May 8, 2021

Hi @MuluhGodson. I pushed commit ad452da, which incorporates your changes into the git submodule theme. Could you kindly rebase this Pull Request and solve the merge conflict with latest changes in unicef:main? Let me know if you need pointers on rebasing if it is a new concept.

@jwflory
Copy link
Member

jwflory commented May 20, 2021

Hi @MuluhGodson, is it possible to rebase your pull request or open a new one instead?

Copy link
Member

@jwflory jwflory left a comment

Choose a reason for hiding this comment

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

I did some work on rebasing this Pull Request on latest changes in main. I added categories to some of the other articles too. Now that the conflicts are resolved, I am merging this in.

Thanks for your work @MuluhGodson! Merged. 🌊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: new change Adds new capabilities or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants