Skip to content

Add optional global nav class-driven font iconography#122

Merged
Phlow merged 2 commits intogh-pagesfrom
unknown repository
Dec 4, 2016
Merged

Add optional global nav class-driven font iconography#122
Phlow merged 2 commits intogh-pagesfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Dec 1, 2016 •
edited by ghost
Loading

Commit name should say it all. I chose class over icon for extensibility and continuity with the footer. Sorry I don't have time to add documentation.

screen shot 2016-12-01 at 3 29 32 pm

screen shot 2016-12-01 at 3 43 14 pm

screen shot 2016-12-01 at 3 44 19 pm

Example use from navigation.yml

- title: Start
  url: "/"
  class: "icon-home"
  side: left

Included with pull

  • Replaces the tree font icon with xing for hacker appeal
  • Also replaces hamburger helper text "Navigation" with "Nav" to support longer site titles

ghost
ghost commented Dec 1, 2016
View reviewed changes
_sass/_07_layout.scss
-webkit-box-shadow: 0 2px 2px 0 rgba(0,0,0,.2);
box-shadow: 0 2px 3px 0 rgba(0,0,0,.2);

[class^='icon-']:before, [class*=' icon-']:before {
Copy link
Author

@ghost ghost Dec 1, 2016 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

Thanks to @TWiStErRob for this one. Modified from twisterrob.github.io.

@ghost
Copy link
Author

ghost commented Dec 2, 2016

FYI tested this on Android and iOS physical devices in addition to emulator. Real device testing reveals a possible bug on iOS devices. Adding screenshots for each case...

@ghost
Copy link
Author

ghost commented Dec 2, 2016 •
edited by ghost
Loading

Mobile emulator for iPhone 6 in Chrome Dev Tools:

screen shot 2016-12-01 at 3 43 14 pm

Actual experience on iPhone 6 (not 6 Plus) without adjustment to Settings > Accessibility:

screen shot 2016-12-02 at 8 57 06 am

Android experience. Device and settings unknown. Sent from client.

screen shot 2016-12-02 at 8 57 25 am

Not sure what's causing the clipping, but didn't see this bug prior to these changes.

@Phlow
Copy link
Owner

Phlow commented Dec 4, 2016

Looks good. But why did you choose the Xing-icon? This is an icon for the biggest German social network for business.

@Phlow Phlow merged commit 6d33ecd into Phlow:gh-pages Dec 4, 2016
ghost
ghost commented Dec 4, 2016
View reviewed changes
_includes/_navigation.html
<ul class="title-area">
<li class="name">
<h1 class="show-for-small-only"><a href="{{ site.url }}{{ site.baseurl }}" class="icon-xing"> {{ site.title }}</a></h1>
<h1 class="show-for-small-only"><a href="{{ site.url }}{{ site.baseurl }}" class="icon-tree"> {{ site.title }}</a></h1>
Copy link
Author

@ghost ghost Dec 4, 2016

Choose a reason for hiding this comment

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

LGTM. Thanks for the heads-up on the German social sharing site.

@Phlow
Copy link
Owner

Phlow commented Dec 4, 2016

Yeah, it's Xing.com – it is the big German counterpart of LinkedIn.

rollingrock pushed a commit to rollingrock/joelucky that referenced this pull request Aug 31, 2025
@Phlow
Merge pull request Phlow#122 from jhabdas/gh-pages
5e52cfc 
Add optional global nav class-driven font iconography
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Phlow