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 condition to only show "RA Section" when there's content available #68

Merged
merged 5 commits into from
Mar 16, 2022
Merged

Add condition to only show "RA Section" when there's content available #68

merged 5 commits into from
Mar 16, 2022

Conversation

zaa6
Copy link
Contributor

@zaa6 zaa6 commented Mar 11, 2022

This commit partially fixes issue #30. The reason I've submitted it as a partial fix is because, the referenced issue doesn't seem to just affects pages inside this repository but also pages inside the cohorts directory located inside this themes' parent directory .i.e. unicef/inventory. I will submit a separate pull request on the parent directory to fix the issue there.

@jwflory jwflory self-assigned this Mar 11, 2022
@jwflory jwflory added the T: bug Something isn't working label Mar 11, 2022
@jwflory
Copy link
Member

jwflory commented Mar 11, 2022

Hey @mmldco, thanks for submitting this. I'll review it shortly.

Also, as a note, it is best practice to first comment on an issue that you are interested in working on so it can be assigned to you. I think @Yavnikaa had started to work on this per her comment in #30, and skipping the step of commenting in an issue often causes duplicated efforts. We'd like to avoid that, especially with more folks coming to find this project in the coming weeks.

@Yavnikaa
Copy link
Contributor

Ahh, so shall I take up some other issue? So that neither of our effort goes waste.
@jwflory can you allot me some other good first issue to begin with?

@jwflory
Copy link
Member

jwflory commented Mar 11, 2022

@Yavnikaa Yeah, sorry about that. 😅 I think any of the issues I opened last night could be good ones. #62 is an especially needed bug fix, and #63, #64, #65, #66, and #67 are feature additions. I'm also hoping to get more added in the coming week.

@zaa6
Copy link
Contributor Author

zaa6 commented Mar 11, 2022

@jwflory I completely understand. @Yavnikaa my sincerest apologies.

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 see where you are going with this, and it does fix the issue on some index pages for the categories. But it does not fix the problem on content pages. In effect, the Related Articles header is only hidden if a page has no content like some index pages. On pages where there is content, the Related Articles header still appears.

A better approach for this would be to use a check if the list of articles from the category is empty, and if it is empty, do not show this HTML <section>. My first suspicion is this is being checked in the $taxonomy variable, which is iteratin through the .Site.Taxonomies.categories list. I could be wrong though.

My idea for a fix might be to play around with the order of how this loop is constructed and to move some HTML elements later on in the loop after checking whether the taxonomy list is empty. Hugo functions should provide useful context to the existing script block here and how it could be improved to fix this bug.

Does this feedback sense? Are you up for working on this further?

Also, for the Cohorts section, don't pay too much mind to that… the code for the Cohorts category is in the unicef/inventory repo, but eventually, we want to consolidate it into the unicef/inventory-hugo-theme repo (see #35). But that is more complex and will probably take time. I wouldn't advise starting on that yet. So don't worry if your changes don't appear correctly in the Cohorts category just yet.

@zaa6
Copy link
Contributor Author

zaa6 commented Mar 11, 2022

I see. I thought the initial solution was too good/simple to be true lol!. Yes! I'm still up for the task. Let me get started.

@zaa6
Copy link
Contributor Author

zaa6 commented Mar 11, 2022

Based on your hunch, I moved the logic for checking whether any articles exist under a category outside the <section> tag. This ensures that the <section> tag in both the index and content pages is only shown if any articles are found. My "fix" however seems to introduce another bug. In the pages were articles are found the <section> is duplicated and appears twice. I'm probably going to revert this commit once I have a properly tested and working commit.

@zaa6
Copy link
Contributor Author

zaa6 commented Mar 12, 2022

The latest commit should fix the both #30 and the new bug introduced by commit e4a8438.

@zaa6
Copy link
Contributor Author

zaa6 commented Mar 12, 2022

After some further testing I'm realising that the above fix seems to only work for top level index and content pages. The fix doesn't seem to work as expected when it comes to nested pages. I'm currently trying to figure out whether the issue is what I'm targeting with the fix or how pages on the different levels have been tagged. I'll report back once I have some answers.

@jwflory
Copy link
Member

jwflory commented Mar 14, 2022

@mmldco Not sure what you mean by nested pages. Could you please clarify? I tried pulling your changes locally and it seems like they are working right in the example site and the Open Source Inventory, but I didn't look too closely.

@zaa6
Copy link
Contributor Author

zaa6 commented Mar 16, 2022

@jwflory I was to categories such as 'Meta' which has sub-categories (nested pages). That being said I've undone the code refactoring I introduced in commit 3f534dd. I was trying to enhance the logic but it seems like it was breaking something. You can proceed to close this issue as the latest commit fixes it. I'll keep making changes locally to see whether I can improve on it. If I'm successful, I'll create a separate pull request with details of the specific problem it addresses.

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.

Hey @mmldco, this looks good! I tested it locally with nested pages, single layout pages, and list layout pages. I'm going to merge this in now. 🎬 Thanks for working on this.

@jwflory jwflory merged commit 5f31b79 into unicef:main Mar 16, 2022
jwflory added a commit to unicef/coach that referenced this pull request Mar 18, 2022
This brings in the following changes:

* 🆕 💄 layouts(shortcodes): Create table layout
  (unicef/inventory-hugo-theme#70)
* 🆕 ✨ layouts(shortcodes): Add embed-pdf shortcode
  (unicef/inventory-hugo-theme#56)
* 🐛 layouts(default): Add condition to show "RA Section" when content is available
  (unicef/inventory-hugo-theme#68)
* 💄 assets(css): Make Table of Contents more appealing and responsive
  (unicef/inventory-hugo-theme#73)
* 🐛 assets(css): Fix styling of highlight code blocks
  (unicef/inventory-hugo-theme#72)

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>
jwflory added a commit to unicef/inventory that referenced this pull request May 9, 2022
This incorporates several of the new contributions made by Outreachy
applicants during the community contribution period of the internship:

* @Neha9849
* @mmldco
* @Yavnikaa
* @BeeBombshell
* @AbihaFatima

The following changes are now incorporated into O.S. Inventory site:

* unicef/inventory-hugo-theme#56 Added embed-pdf shortcode
* unicef/inventory-hugo-theme#68 Add condition to only show "RA Section" when there's content available
* unicef/inventory-hugo-theme#73 Make Table of Contents more appealing and responsive
* unicef/inventory-hugo-theme#72 Improved styling of highlight code blocks
* unicef/inventory-hugo-theme#83 Added shortcodes for CAUTION and IMPORTANT admonition
* unicef/inventory-hugo-theme#94 Highlight the background color of buttons while hovering

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>
jwflory added a commit to unicef/inventory that referenced this pull request May 9, 2022
This incorporates several of the new contributions made by Outreachy
applicants during the community contribution period of the internship:

* @Neha9849
* @mmldco
* @Yavnikaa
* @BeeBombshell
* @AbihaFatima

The following changes are now incorporated into O.S. Inventory site:

* unicef/inventory-hugo-theme#56 Added embed-pdf shortcode
* unicef/inventory-hugo-theme#68 Add condition to only show "RA Section" when there's content available
* unicef/inventory-hugo-theme#73 Make Table of Contents more appealing and responsive
* unicef/inventory-hugo-theme#72 Improved styling of highlight code blocks
* unicef/inventory-hugo-theme#83 Added shortcodes for CAUTION and IMPORTANT admonition
* unicef/inventory-hugo-theme#94 Highlight the background color of buttons while hovering

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>
Neha9849 added a commit to Neha9849/inventory-hugo-theme that referenced this pull request Jun 1, 2022
fixing the pdf view

minor fix

Update exampleSite/content/installation/elements/_index.en.md

Co-authored-by: Justin W. Flory <jflory7@gmail.com>

:lipstick: layouts(navigation): Use consistent color on all pages (unicef#58)

This commit fixes a bug where the UNICEF Blue was hard-coded into the
navigation bar, and changes that hard-coded value to use the primary
color set by the site maintainer in the Hugo config file (`params.`
`primary_color`).

Additionally, I changed the color for the translations options to use a
white color, since it is hard to read when the primary color is a darker
color.

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

:new: :lipstick: layouts: Add dpg-report layout (see unicef/coach#1) (unicef#44)

This commit creates a new custom layout for DPG report cards in the
UNICEF Inventory theme. DPG report cards are at-a-glance views of a
Venture Fund company's progress in meeting the DPG Standard.

More context about this change is in unicef/coach#1. This commit should
be merged once the idea is validated and the early prototyping phase is
complete.

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

:new: :lipstick: layouts(shortcodes): Create table layout (unicef#70)

Shortcode for a table used in unicef/inventory#125.

:new: :sparkles: layouts(shortcodes): Add embed-pdf shortcode (unicef#56)

I have added a demo of this shortcode on the `installation/elements` page. For using this Shortcode, we have to pass the name of the pdf into the shortcode as shown in the demo. The pdfs should be stored in the `static/pdfs` folder.

Fixes unicef#33.

Co-authored-by: Justin W. Flory <jflory@unicef.org>

:bug: layouts(default): Add condition to show "RA Section" when content is available (unicef#68)

Fixes unicef#30.

* Add hugo condition to only show "RA Section" when there's content available
* Modify hugo conditions to check for available content before displaying the ra section
* Fix duplication of 'RA' section bug introduce by commit e4a8438
* Refactor hugo condition reposible for fixing the bugs in both issue unicef#30 and commit e4a8438
* Undo code refactoring introduced by commit 3f534dd

Co-authored-by: Zab <zab@marmalade.co.ke>

:lipstick: assets(css): Make Table of Contents more appealing and responsive  (unicef#73)

* Complete the stated task
* Make table of contents more appealing and responsive
* Update template title
* Update layouts/partials/head.html

Commit emoji: https://gitmoji.dev/

Co-authored-by: Justin W. Flory <jflory@unicef.org>

🐛 assets(css): Fix styling of highlight code blocks (unicef#72)

* 🐛 fix: Improved styling of highlight code blocks
* Reduced whitespace between codelines using css
* 📝 docs: Added comment for CSS changes

:recycle: layouts(dpg-report): Reuse download-pdf partial (unicef#74)

The PDF download button logic was moved into a partial after this layout
was created. This refactors to use the partial and maintain less code.

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

:bug: :wrench: config: Removed Duplicate Home Link in Footer (unicef#80)

Fixes unicef#57.

In Footer partial, we have this:

```html
 <li><a class="nav-link text-white" href="{{ site.BaseURL | relLangURL }}">HOME</a></li>
        {{ range site.Menus.main }}
          {{if ne .Name "pages"}}
            <li class="nav-item">
              <a class="nav-link text-white text-uppercase" href="{{ .URL | absLangURL }}">{{ .Name }}</a>
            </li>
          {{ end }}
        {{ end }}
```

In all other upstreams, we don't have Home in the Menu array but we do have it in the exampleSite. This is what causes the duplication in exampleSite. So, I have removed it from the config file of the example site.

:bug: :memo: README: Fix link for example site config (unicef#82)

An example configuration used to be bundled at the top of the repo, but
now it is included as part of the example site used in building this
theme. This commit fixes the broken link in the README.

Closes unicef#79.

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

:bug: :wrench: ci: Fix GH Pages push with string comparison operator

This commit changes the string comparison operator for the git user name
in the deploy script used in continuous integration. Currently the check
is checking if the string is not empty, then it overrides what was set.
We actually wanted the inverse.

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

:new: :memo: .github: Add contributing guidelines (unicef#86)

* 🆕 📝 .github: Add contributing guidelines

This commit adds new contributing guidelines for the UNICEF Inventory
theme. It includes four main topics:

1. Contribution process (a.k.a. governance)
2. Conventions & courtesies
3. Structure & components
4. How to create a developer environment

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

* Address @Idadelveloper feedback in PR unicef#86; s/tickets/issues/

This commit makes a few changes, mostly coming from Ida's feedback in
Pull Request unicef#86:

1. Add a note about creating a git feature branch after being assigned
   an issue.
2. Add a note to tag an issue number in the commit message(s) of a Pull
   Request.
3. Change all mentions of "ticket" to "issue" to be more clear.

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

💄 Admonitions: Standardize appearance and add new shortcodes (unicef#83)

* 💄 fix: Added CAUTION admonition shortcode (closes unicef#66)
* 💄 fix: Added IMPORTANT admonition shortcode (closes unicef#64)
* 💄 fix: Added admonition icons and updated docs

* Revert IDE changes to French localized file.

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

* Remove extraneous changes to index page

Signed-off-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

Co-authored-by: Justin W. Flory (he/him) [UNICEF Innovation] <jflory@unicef.org>

:lipstick: css: Highlight buttons on hovering (unicef#94)

Fixes unicef#90.

Type of Change:

- Code
- User Interface

📝 docs: Added table shortcode in elements page (unicef#96)

Fixes unicef#71.

This PR adds table shortcodes to the existing tables along with the documentation to the elements page.

added inventory layouts

 Added layouts of inventory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If no featured articles exist, do not show "Related Articles" header at bottom of page
3 participants