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 simple figure style for content class #130

Merged
merged 4 commits into from Jan 7, 2019

Conversation

Projects
None yet
3 participants
@piojanu
Copy link
Contributor

piojanu commented Dec 27, 2018

Prerequisites

Put an x into the box(es) that apply:

  • This pull request fixes a bug.
  • This pull request adds a feature.
  • This pull request introduces breaking change.

Description

It adds simple figure style so this gohugo shortcode:
{{< figure src="/images/N90.jpg" caption="N90 nebula, \"New stars shed light on the past\" by ESA/Hubble" >}}
produces this result:
screenshot 2018-12-27 at 13 37 11

Checklist

Put an x into the box(es) that apply:

General

  • Describe what changes are being made
    Change content's css.
  • Explain why and how the changes were necessary and implemented respectively
    Before it using gohugo shortcode for figures was giving bad formatted/bad looking figures with caption.
  • Reference issue with #<ISSUE_NO> if applicable

Resources

  • If you have changed any SCSS code, run make release to regenerate all CSS files

Contributors

  • Add yourself to CONTRIBUTORS.md if you aren't on it already
@luizdepra

This comment has been minimized.

Copy link
Owner

luizdepra commented Dec 31, 2018

Nice, thank you for the PR.

But, could you add the figure snippet into exampleSite/content/posts/theme-demo.md?

@piojanu

This comment has been minimized.

Copy link
Contributor

piojanu commented Jan 5, 2019

Okay! Should I build the example website somehow?

@artspb

This comment has been minimized.

Copy link
Contributor

artspb commented Jan 5, 2019

I'm really sorry to pop-up in the middle of the conversation. But is it really necessary to use a 9 MB image to show the feature? I see the following downsides:

  1. Once added to git it'll never be removed. Everyone who wants to use this wonderful theme will download it as a part of the example site.
  2. Loading of the example site may take a while if the Internet connection is not stable. For mobile devises it additionally means draining of an accu.
  3. I'm probably missing something but it looks like that the same demonstration effect can be achieve with say a 300 KB picture.

Once again, I'm sorry for bothering you with my comment. I understand that my concerns may be irrelevant for many if not all. I also like the feature and will definitely use it, thank you for implementing it.

@piojanu

This comment has been minimized.

Copy link
Contributor

piojanu commented Jan 5, 2019

@artspb you're making a really good point. I compressed the image.

@piojanu piojanu force-pushed the piojanu:master branch from 018918b to 5d715e6 Jan 5, 2019

@@ -39,6 +39,17 @@
}
}
}
figure {
border: thin silver solid;

This comment has been minimized.

@luizdepra

luizdepra Jan 7, 2019

Owner

Just to keep styling consistent... It looks better without the border. So, please, remove this line.
And also change margin and padding to 0.

figcaption p {
text-align: center;
font-style: italic;
font-size: smaller;

This comment has been minimized.

@luizdepra

luizdepra Jan 7, 2019

Owner

Better use 1.6rem as the font-size here.

@luizdepra

This comment has been minimized.

Copy link
Owner

luizdepra commented Jan 7, 2019

I just request two minor changes. You will need to rebase yout fork and regenerate the resources too.

@piojanu piojanu force-pushed the piojanu:master branch from 5d715e6 to 9407181 Jan 7, 2019

@piojanu

This comment has been minimized.

Copy link
Contributor

piojanu commented Jan 7, 2019

@luizdepra Done :)

@luizdepra

This comment has been minimized.

Copy link
Owner

luizdepra commented Jan 7, 2019

Nice, thank you.

@luizdepra luizdepra merged commit 12b4246 into luizdepra:master Jan 7, 2019

1 check passed

deploy/netlify Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment