Skip to content

Conversation

@jamesob
Copy link
Contributor

@jamesob jamesob commented Jul 11, 2018

  • Add a horizontal logo
  • Reposition a few elements on the home page

Before

selection_061

After

selection_060

Copy link
Collaborator

@harding harding left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! One noteworthy comment below about the page header image size on mobiles, and a several other comments about minutia that you should feel free to ignore.

}

a.site-title {
width: 30%;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you want to use a width defined in either px that displays at the same size everywhere or in em that only changes when the user changes the font size. Otherwise, this can be a bit wussy-looking on narrow mobile screens:

2018-07-12-03_54_48_466853716

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks!

margin-top: 2em;

> .logo {
width: 30%;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you put min-width: 10em; below the existing width line, the graphic will be shown a bit larger on narrow mobiles but will be the same size on the widest displays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,18 @@
---
# Forked from the default minima layout to include our main.css
Copy link
Collaborator

Choose a reason for hiding this comment

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

It already did that on my system, were you having problems? I don't see any layout-affecting differences when diffing a copy of the _site dir generate with this file to one generated without it, nor do I on a visual inspection of the rendered site.

OTOH, even if creating this file now is unnecessary, it's likely something that we'll end up customizing later anyway, so I'm not opposed to this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I see, the CSS file was moved from its default location in d091a44 and only the homepage layout was updated to reflect that, so without this file, the newsletters don't get style changes.

This is fine. An alternative is to move the css file back to its original location in /assets/ and drop the special code from the home layout. I don't think it matters long term if this site is continued to be customized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup; I prefer to keep _layout/page.html bundled as it's convenient to have the code ready at hand for modification, as well as for easier discovery - had to google around a bit to figure out where the page template lived beforehand.

index.md Outdated
{:style="text-align: center"}
![OpTech Logo](/img/logos/optech.png)

<div class="logo">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have any problem with using HTML here (or anywhere, really), but FYI you can apply a CSS class to a block element in Kramdown like this:

{:.logo}
![OpTech Logo](/img/logos/optech.png)

That will create:

<p class="logo"><img src="/img/logos/optech.png" alt="OpTech Logo" /></p>

Other things you can do:

  • {#custom-id}
  • {:custom-tag: "custom-value}
  • Plus other fancier stuff, see the manual

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, neat! Fixed.

margin-bottom: 2em;
}

div#sponsors {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this vertical whitespace looks good over the top of the Sponsors <h3>, you might want to consider applying it to every <h3> on the website, both for consistency and for the avoidance of having too many ad hoc rules that may making figuring out style things later harder.

@jnewbery
Copy link
Contributor

Requires rebase (and addressing harding's comments).

- Add a horizontal logo
- Reposition a few elements on the home page
@jamesob
Copy link
Contributor Author

jamesob commented Jul 12, 2018

I've incorporated @harding's good feedback (thanks, Dave) and I've changed the inline logo to remove the text, as it felt like we were repeating "Bitcoin Optech" a bit too much graphically. I can revert if you guys don't like this. The new PNG also has a transparent background (instead of the slightly-off white).

selection_064

selection_065

@harding
Copy link
Collaborator

harding commented Jul 12, 2018

0c25896 LGTM.

@jnewbery
Copy link
Contributor

The new PNG also has a transparent background

Thanks! That was bugging me 🙂

@jnewbery
Copy link
Contributor

0c25896 LGTM 🚀

@jnewbery jnewbery merged commit 034a44f into master Jul 12, 2018
@jamesob jamesob deleted the layout-tweaks branch July 12, 2018 19:15
harding referenced this pull request in harding/bitcoinops.github.io Oct 22, 2018
harding referenced this pull request in harding/bitcoinops.github.io Oct 22, 2018
harding referenced this pull request in harding/bitcoinops.github.io Oct 22, 2018
jnewbery added a commit that referenced this pull request Oct 23, 2018
harding referenced this pull request in harding/bitcoinops.github.io Oct 23, 2018
jnewbery added a commit that referenced this pull request Oct 24, 2018
@harding harding mentioned this pull request Feb 10, 2020
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants