-
Notifications
You must be signed in to change notification settings - Fork 66
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 ability to reference global resources #84
Conversation
That's possibly because the file uses tabs for indentation and my editor uses spaces. Would you like to keep the tabs? If you don't mind using spaces, I can convert the entire file to spaces (usually more robust and avoids such issues). |
Thanks for the offer, but I prefer to stay with tabs. Not that I care that much, but changing every line would disrupt the git history/blame feature. |
I have now tested it and it works fine. But there is one problem: If both parameters, match and globalMatch, are present, the images that belong to the match parameter are displayed, hover, the metadata is missing because of line 106-108. Could you rebase your commit, use tabs as intention and fix this? Then I would be happy to merge this PR. |
@mfg92 I think I have fixed the bug. I think I have also fixed the indentation issue. Finally, I added some details about this new option in the README. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix and the README update. I do have some feedback for you though.
layouts/shortcodes/gallery.html
Outdated
|
||
{{/* Get metadata from sidecar file, if present. Else an empty dictionary is used. */}} | ||
|
||
{{/* Get metadata from sidecar file, if present. Else an empty dictionary is used. */}} | ||
{{ $metaFileName := print $original.Name ".meta"}} | ||
{{ $metadata := $currentPage.Page.Resources.GetMatch ($metaFileName) }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If $globalMatch is present, this line does unnecessary work. I think the solution is an if ... else ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for raising this. I used to have it with if and else, but for some reason I couldn't get it to work. Let me have another look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mfg92 This is now fixed.
* Added ability to reference "global" resources * Updated README to include info about the globalMatch. --------- Co-authored-by: mfg92 <[email protected]>
This is now part of of v1.3.0. Thanks a lot for your contribution. |
No problem; thank you very much for maintaining a useful shortcode. |
* Added ability to reference "global" resources * Updated README to include info about the globalMatch. --------- Co-authored-by: mfg92 <[email protected]>
When a user needs to embed images across different pages, having duplicate images locally in each page's
images
directory is redundant. I wanted to add a gallery of the same images across different pages/sections of the website. I couldn't find a neat way to do this currently. It appears that the shortcode would only look locally in each page's resources.I added an extra option,
globalMatch
, that will look for images under the/assets
directory instead, leveraging the global resources utility from Hugo. This to work, a user will need to place those “repeating” images under/assets
(images could exist under a subdirectory underassets
).Example
content/gallery/index.md:
content/news/something/index.md:
Both pages would now embed the same images, yet those images exist only once on the server.
I hope this is useful to others.
Rafael