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

Added embed-pdf shortcode #56

Merged
merged 5 commits into from
Mar 15, 2022
Merged

Added embed-pdf shortcode #56

merged 5 commits into from
Mar 15, 2022

Conversation

Neha9849
Copy link
Member

@Neha9849 Neha9849 commented Feb 23, 2022

I have added demo of this shortcode in installation/configuration page .
For using this Shortcode we have to pass the Name of the pdf into the shordcode as shown in the demo.
The pdfs should be stored in the static/pfds folder.
Fixes #33.

@Neha9849 Neha9849 force-pushed the main branch 2 times, most recently from bd7aa9b to 032e97f Compare February 23, 2022 16:13
@Neha9849
Copy link
Member Author

@jwflory I am unable to understand why checks failed. Can you check into this issue

@jwflory jwflory self-assigned this Feb 23, 2022
@jwflory jwflory added the T: new change Adds new capabilities or functionality label Feb 23, 2022
@jwflory
Copy link
Member

jwflory commented Feb 23, 2022

Thanks for this PR @Neha9849! I'll review on Thursday (tomorrow).

One thing in the meantime though: could you please rebase your fork's main branch on the latest changes in the main branch in this repo? This will get rid of the duplicate commits that are appearing in this PR. If you have never rebased before, see these two articles for help:

I suggest creating a new git branch from your changes in your fork's main branch first, in case you need a backup if things go wrong. (It can happen!) If you need additional guidance on rebasing, don't hesitate to ask for help.

P.S. — Using git feature branches will prevent things like this from happening most of the time. And when it does happen, rebasing from a feature branch will be easier than developing on your fork's main branch. 🙂

@Neha9849 wrote…
@jwflory I am unable to understand why checks failed. Can you check into this issue

Oof, this is annoying. The logs are not helpful when HTML-Proofer throws an error. This is related to #54 / CircleCI-Public/hugo-orb#51. When I review this PR tomorrow, I'll look more closely at why the check is failing.

My guess is that HTML-Proofer cannot locate the URL of the PDF and thinks it is a broken URL, so the check fails. If this is the case, we need to use a permalink of some kind for the PDF path, or the HTML-Proofer configuration needs to be adjusted so it won't fail when linking to internal content. Either way, I'll dig in more tomorrow…

added download-pdf partial

Keep HTML element on a single line, and other minor spacing

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

added embed-pdf shortcode

added demo in installation/configuration file
@Neha9849
Copy link
Member Author

From next onwards i will use branches. I did the rebase as given in the article, i think i did it right. I learnt some cool stuff thanks :)

@jwflory
Copy link
Member

jwflory commented Feb 28, 2022

Hey Neha, the git rebase looks good. Glad you got it figured out! I ran out of time to review this before I went on leave last Friday. When I'm back in office next week, I'll review this. Thanks for being patient!

@jwflory
Copy link
Member

jwflory commented Feb 28, 2022

One idea in the meantime you can try is to run HTML-Proofer locally in the command line on the generated Hugo HTML files. HTML-Proofer is a Ruby gem. Not sure if you done much development in Ruby before or have an environment. But if it is not much trouble, you can install the gem and use the same command that the CI pipeline uses to see the results on your local machine.

Generally the process I use to debug is to first run hugo in the exampleSite/ directory. Then, the HTML output is built to exampleSite/public/. Then you can invoke HTML-Proofer from the command line on the generated HTML directory. You can find the needed parameters and arguments for HTML-Proofer in the CircleCI build linked from this pull request. But to see the errors, you need to remove the --error-sort flag (or I think that's the parameter name).

I hope this makes sense. I'm typing it all from my phone and not able to verify it from my end. If you can't figure this out, don't worry about it. If you do get the errors from HTML-Proofer to print, it would be great to post them here for later debugging. I suspect it is something that needs to be addressed separately from this PR, but I could be wrong. It could be that some URL has simply broken.

@Neha9849
Copy link
Member Author

Neha9849 commented Mar 6, 2022

One idea in the meantime you can try is to run HTML-Proofer locally in the command line on the generated Hugo HTML files. HTML-Proofer is a Ruby gem. Not sure if you done much development in Ruby before or have an environment. But if it is not much trouble, you can install the gem and use the same command that the CI pipeline uses to see the results on your local machine.

I am not familiar with Ruby and dont have the environment setup :(

I hope this makes sense. I'm typing it all from my phone and not able to verify it from my end. If you can't figure this out, don't worry about it. If you do get the errors from HTML-Proofer to print, it would be great to post them here for later debugging. I suspect it is something that needs to be addressed separately from this PR, but I could be wrong. It could be that some URL has simply broken.

Oops sorry for the delay 😅 but thanks for replying even through phone. I think it has something to do with the Link which i used to refer the Pdf.

@jwflory
Copy link
Member

jwflory commented Mar 9, 2022

Hey @Neha9849, thanks for your patience! I ended up catching a cold and took the last two days off as sick days. Just getting caught up here now.

@Neha9849 wrote…
I am not familiar with Ruby and dont have the environment setup :(

No worries at all!

@Neha9849 wrote…
Oops sorry for the delay 😅 but thanks for replying even through phone. I think it has something to do with the Link which i used to refer the Pdf.

Actually, I am surprised. Both of our hypotheses were wrong 😂 Here is the log from HTML-Proofer when I ran the command locally:

htmlproofer exampleSite/public --allow-hash-href --alt-ignore --checks-to-ignore --check-html --disable-external --empty-alt-ignore --http-status-ignore 0,999 --timeframe 6w --url-ignore /inventory-hugo-theme/

Running ["ImageCheck", "HtmlCheck", "ScriptCheck", "LinkCheck"] on ["public"] on *.html... 

- public/installation/configuration/index.html
  *  553:12: ERROR: Missing semicolon after character reference '&lt'.
        <b>&lt</b>
           ^ (line 553)
  *  557:17: ERROR: Missing semicolon after character reference '&gt'.
            <b> &gt</b>
                ^ (line 557)

HTML-Proofer found 2 failures!

I'm still digging into this PR. I think this will be an easy fix in the layout file.

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 @Neha9849, looking good! 👍🏻 There are some quick fixes here to get the CI pipeline passing. But more importantly, it seems like the PDF appears at a fixed size and cannot be moved inside the container. It seems there are some manual controls being done here to make the PDF appear nicely in the theme. We could try to maintain this customization, but if it doesn't work out, perhaps there is an easy way for us to revert to using the default browser controls for downloading and changing pages of the PDF.

Let me know if this feedback makes sense or you have any follow-up questions. Thanks for working on this.

layouts/shortcodes/embed-pdf.html Outdated Show resolved Hide resolved
layouts/shortcodes/embed-pdf.html Outdated Show resolved Hide resolved
layouts/shortcodes/embed-pdf.html Outdated Show resolved Hide resolved
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.

Thanks @Neha9849! Your changes look good and the PDF renders correctly now. A screenshot from a responsive display is below.

Before merging this, I wanted to make an additional note in the documentation on where to place PDFs. See below. After that, I think this is ready to be merged! 🎬

Screenshot of embedded PDF from responsive screen size. Full PDF is shown.

layouts/shortcodes/embed-pdf.html Show resolved Hide resolved
Co-authored-by: Justin W. Flory <jflory7@gmail.com>
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.

Great work here @Neha9849! This is ready to merge. ✅ I'll work on getting this added downstream to the Open Source Inventory soon too.

@jwflory jwflory merged commit f237889 into unicef:main Mar 15, 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: new change Adds new capabilities or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support embedded PDFs in an article
2 participants