Refactor social links #141

ashawley · 2017-07-12T21:47:03Z

This makes the required consolidation of social links to a single file as suggested in #112.

It required removing the use of the templates on the about page. Hopefully, others aren't depending on it.

ashmaroli · 2017-07-13T01:20:44Z

It required removing the use of the templates on the about page. Hopefully, others aren't depending on it.

The about.md here is only for demonstration on GitHub Pages. It's not included in the release. The namesake release-version has already been fixed released with Jekyll 3.5.0.

ashmaroli · 2017-07-13T01:34:53Z

Tested this locally. No major issues.

Just one personal nit:
viewBox is not a valid css property (couldn't find it on caniuse.com)

Also, it'd be great if the README.md documents these new config keys.
Thanks for opening this PR 👍

ashawley · 2017-07-13T02:42:58Z

viewBox is not a valid css property

Ok, I wouldn't know the reason for it so I can' defend it, but the SVG icons still work now after it's removed. 👍

Also, it'd be great if the README.md documents these

Ok, I've taken a first pass at documenting them in a new section in the README.

ashmaroli · 2017-07-13T03:10:20Z

README.md

+instagram_username: ig
+linkedin_username: linkedin
+pinterest_username: pinterest
+stackoverflow_username: stackoverflow


since this is documentation, I think we'll have to point to the right way as well.
e.g. StackOverflow profiles are located at users/ i.e. for username: minima, the profile is actually at stackoverflow.com/users/[random]/minima

Agreed, I'll try to improve them.

I forgot that Stack Overflow was like this. Unfortunately, the premise of these social links is that the username doubles as both the link and the link text. :-(

We could of course minimize to this:

stackover_username: 111111/jekyll

But the link text would still be 111111/jekyll unless it was filtered out. Maybe punt on Stackoverflow for now? See if people actually want it? Stack Overflow have their own flair badges anyway, see: https://stackoverflow.com/users/flair

I'm not overly concerned about it. Just mentioned it as an afterthought.. 😃

ashawley · 2017-07-19T18:31:15Z

Since the link text is goofed up, I'm going to drop Stackoverflow unless there are any objections...

ashmaroli · 2017-07-20T02:12:10Z

@ashawley, what do you think about changing the format entirely?

Convert social-links into a horizontal "social-bar"
- last section of the footer
- full width
Drop the names but instead increase the icon-size a bit
The names can be added as the title attribute for the links/icons.

this is in line with minimalism and also will minimally alter the footer height with addition of multiple icons.

ashawley · 2017-07-20T02:22:19Z

As a way to get around the stackoverflow issue? I guess that could work.

I may not have the design expertise to make it work.

I've rebased and resolved the latest conflicts.

Do you want to fork this branch and give it a try in yet another PR?

ashmaroli · 2017-07-20T02:28:17Z

I started a discussion for Minima 3.0 to include breaking changes (such as the idea I proposed above)

Do you want to fork this branch and give it a try in yet another PR?

That could lead to messy git history..
IMO, you may continue refining this PR as you see fit.

I'll start fresh.

ashawley · 2017-07-20T02:45:31Z

That could lead to messy git history..

I'm just trying to get people's contributions across the finish line. I'm happy to clean up git history if that's what keeping this from getting merged.

ashmaroli · 2017-07-20T02:50:33Z

I'm just trying to get people's contributions across the finish line

You misunderstood me.
This PR is good enough. But I'm starting fresh because I'm introducing structural changes to the footer which may not be approved immediately. Like i said it's targeted for a future 3.0 version

Hence starting fresh.

ashmaroli

Found a few areas where you can optimize further

ashmaroli · 2017-07-20T15:58:48Z

_includes/social.html

@@ -0,0 +1,14 @@
+<ul class="social-media-list">
+  {% if site.dribbble_username %}<li><a href="https://dribbble.com/{{ site.dribbble_username | cgi_escape | escape }}"><svg class="svg-icon" xmlns="http://www.w3.org/2000/svg"><use xlink:href="/assets/icons.svg#dribbble"></use></svg> <span class="username">{{ site.dribbble_username | escape }}</span></a></li>{% endif %}


you can cleanup further by removing xmlns="http://www.w3.org/2000/svg. It will be used from icons.svg anyways

This I can do

ashmaroli · 2017-07-20T16:04:04Z

about.md


 You can find the source code for Jekyll at
-{% include icon-github.html username="jekyll" %} /
-[jekyll](https://github.com/jekyll/jekyll)
+[github.com/jekyll/jekyll](https://github.com/jekyll/jekyll)


It doesn't make a difference, but please replace this with text from Jekyll 3.5's about.md template

Ok, I'll look in to changing the about page to be in line with jekyll.

ashmaroli · 2017-07-20T16:08:34Z

assets/icons.svg

@@ -0,0 +1,27 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+
+<symbol id="dribbble" fill-rule="evenodd" clip-rule="evenodd" stroke-linejoin="round" stroke-miterlimit="1.414"><path d="M8 16c-4.41 0-8-3.59-8-8s3.59-8 8-8 8 3.59 8 8-3.59 8-8 8zm6.747-6.905c-.234-.074-2.115-.635-4.257-.292.894 2.456 1.258 4.456 1.328 4.872 1.533-1.037 2.624-2.68 2.93-4.58zM10.67 14.3c-.102-.6-.5-2.688-1.46-5.18l-.044.014C5.312 10.477 3.93 13.15 3.806 13.4c1.158.905 2.614 1.444 4.194 1.444.947 0 1.85-.194 2.67-.543zm-7.747-1.72c.155-.266 2.03-3.37 5.555-4.51.09-.03.18-.056.27-.08-.173-.39-.36-.778-.555-1.16-3.413 1.02-6.723.977-7.023.97l-.003.208c0 1.755.665 3.358 1.756 4.57zM1.31 6.61c.307.005 3.122.017 6.318-.832-1.132-2.012-2.353-3.705-2.533-3.952-1.912.902-3.34 2.664-3.784 4.785zM6.4 1.368c.188.253 1.43 1.943 2.548 4 2.43-.91 3.46-2.293 3.582-2.468C11.323 1.827 9.736 1.176 8 1.176c-.55 0-1.087.066-1.6.19zm6.89 2.322c-.145.194-1.29 1.662-3.816 2.694.16.325.31.656.453.99.05.117.1.235.147.352 2.274-.286 4.533.172 4.758.22-.015-1.613-.59-3.094-1.543-4.257z"/></symbol>


you can clean up further by removing symbol attributes other than id.
Its better to have viewBox attribute defined per symbol. (Note: reconfirm if the resulting SVG paths are still aligned in the browser after you make the changes)

I'm not sure the consequences of this. This wasn't brought up in the previous review of the original PR. Can someone confirm? Otherwise, we can probably do a follow-up after this is merged.

I couldn't find a valid reference pointing to the dire consequences of this, but irrespective of the decision, if you want a working demo you can checkout #146

To clarify, I don't know SVG enough to know the consequence of the change, and the original poster, @domingohui, is out of radio contact to defend the original use of symbol.

@ashawley Don't bother to manually optimize the SVG, that's whats SVGO is for. No problem if you prefer that we deal with this in another PR.

ashmaroli · 2017-07-21T16:24:18Z

Apart from one other place you need to make an edit, this looks okay to me.
/cc @DirtyF Requesting a review from you.. Thanks.

DirtyF

Made some suggestions. What do you think?

DirtyF · 2017-07-22T00:11:08Z

assets/icons.svg

@@ -0,0 +1,27 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+
+<symbol id="dribbble" fill-rule="evenodd" clip-rule="evenodd" stroke-linejoin="round" stroke-miterlimit="1.414"><path d="M8 16c-4.41 0-8-3.59-8-8s3.59-8 8-8 8 3.59 8 8-3.59 8-8 8zm6.747-6.905c-.234-.074-2.115-.635-4.257-.292.894 2.456 1.258 4.456 1.328 4.872 1.533-1.037 2.624-2.68 2.93-4.58zM10.67 14.3c-.102-.6-.5-2.688-1.46-5.18l-.044.014C5.312 10.477 3.93 13.15 3.806 13.4c1.158.905 2.614 1.444 4.194 1.444.947 0 1.85-.194 2.67-.543zm-7.747-1.72c.155-.266 2.03-3.37 5.555-4.51.09-.03.18-.056.27-.08-.173-.39-.36-.778-.555-1.16-3.413 1.02-6.723.977-7.023.97l-.003.208c0 1.755.665 3.358 1.756 4.57zM1.31 6.61c.307.005 3.122.017 6.318-.832-1.132-2.012-2.353-3.705-2.533-3.952-1.912.902-3.34 2.664-3.784 4.785zM6.4 1.368c.188.253 1.43 1.943 2.548 4 2.43-.91 3.46-2.293 3.582-2.468C11.323 1.827 9.736 1.176 8 1.176c-.55 0-1.087.066-1.6.19zm6.89 2.322c-.145.194-1.29 1.662-3.816 2.694.16.325.31.656.453.99.05.117.1.235.147.352 2.274-.286 4.533.172 4.758.22-.015-1.613-.59-3.094-1.543-4.257z"/></symbol>


@ashawley Don't bother to manually optimize the SVG, that's whats SVGO is for. No problem if you prefer that we deal with this in another PR.

DirtyF · 2017-07-22T00:18:00Z

_config.yml

+linkedin_username: jekyll
+pinterest_username: jekyll
+youtube_username: jekyll
+googleplus_username: +jekyll



I would rather only keep existing Jekyll accounts and leave other services commented to avoid unnecessary 404s.

Good catch. I agree. I had done that originally, but it got reverted while testing. Commenting them out again...

DirtyF · 2017-07-22T00:29:51Z

_includes/social.html

+  {% if site.youtube_username %}<li><a href="https://youtube.com/{{ site.youtube_username | cgi_escape | escape }}"><svg class="svg-icon"><use xlink:href="/assets/icons.svg#youtube"></use></svg> <span class="username">{{ site.youtube_username | escape }}</span></a></li>{% endif %}
+  {% if site.googleplus_username %}<li><a href="https://plus.google.com/{{ site.googleplus_username | escape }}"><svg class="svg-icon"><use xlink:href="/assets/icons.svg#googleplus"></use></svg> <span class="username">{{ site.googleplus_username | escape }}</span></a></li>{% endif %}
+  {% if site.rss %}<li><a href="{{ site.baseurl }}/feed.xml"><svg class="svg-icon"><use xlink:href="/assets/icons.svg#rss"></use></svg> <span>{{ site.rss | escape }}</span></a></li>{% endif %}
+</ul>


Should we rather loop on a social array in the config to apply DRY principle here?

That'd be a breaking change.. which is being covered (differently albeit) in #146

Right, I think the point of this PR is to just add new sites, but not break the existing ones, Twitter and GitHub.

Rather than introduce 10 new top-level config flags that we know we want to break, I'd rather we implement the social hash described in #146, and have a fallback to retain support for the old foo_username syntax for the two existing ones.

That makes sense. Is that possible? What would it take to do? Just a squirrelly if-conditional?

say the hash is assigned to variable called socials, then I think the following will do fine:

{% assign github_user = social.github | default: site.github_username %} <li><a>{{ github_user }}</a></li>

ashmaroli · 2017-07-22T01:21:43Z

about.md

-You can find the source code for the Jekyll new theme at:
-{% include icon-github.html username="jekyll" %} /
+You can find the source code for Minima at GitHub:
+[jekyll][jekyll-organization] /


That approach was changed upstream to allow users switch themes via GH-Pages.

But this about.md is specific to Minima.
It also serves as a showcase page for what Minima looks like when used with jekyll new --

Confusion.. confusion..

ashawley · 2017-08-03T14:29:52Z

Are there any other corrections? It seemed like there was a lot of enthusiasm in the original PR for improving the social links in minima.

petermattia · 2017-08-08T18:28:55Z

Hi,
New Jekyll user here. I know Jekyll is quite popular with academics/grad students. Could you also add a Google Scholar link here? Thanks!

ashawley · 2017-08-08T19:37:52Z

I would think adding more social links wold be possible, but the hope (at least mine is) is that this initiative is on it's way to getting merged and then adding other social sites can be taken up later. The original work on this PR began almost 6 months ago so hopefully improved social links get incorporated.

ashawley · 2017-08-17T14:11:41Z

So the SVG files used to be an include file, but this PR makes it an asset file. Is there a risk of name collisions with an asset called icons.svg? Should it be something like assets/minima/social-icons.svg instead?

ashawley · 2018-01-14T22:52:51Z

I've rebased with the latest master commits and added an entry to the release history file.

Could this get merged, soon? People are requesting other social links, but this is blocking those questions.

The last status report was Oct 25, 2017

This changes the documentation to match the version in _config.yml in jekyll#141.

Refactor social links

This changes the documentation to match the version in _config.yml in jekyll#141.

ashawley force-pushed the domingohui-master branch from 583de67 to 9502938 Compare July 13, 2017 02:42

ashmaroli reviewed Jul 13, 2017

View reviewed changes

ashawley force-pushed the domingohui-master branch 2 times, most recently from 997ee64 to d423070 Compare July 13, 2017 15:49

ashawley force-pushed the domingohui-master branch from d423070 to f5a9da8 Compare July 20, 2017 02:17

ashmaroli mentioned this pull request Jul 20, 2017

Convert social-links column into a row of icons #146

Merged

ashmaroli requested changes Jul 20, 2017

View reviewed changes

DirtyF reviewed Jul 22, 2017

View reviewed changes

DirtyF changed the title ~~Generalize and reorganize social media links #112~~ Refactor social links Jul 22, 2017

This was referenced Jul 22, 2017

Social Media Icons / Links #39

Closed

Generalize and reorganize social media links #39 #112

Closed

ashmaroli reviewed Jul 22, 2017

View reviewed changes

ashawley force-pushed the domingohui-master branch from d4b4184 to 65aa56b Compare July 22, 2017 13:19

ashmaroli approved these changes Aug 3, 2017

View reviewed changes

ashmaroli mentioned this pull request Aug 11, 2017

removes unwanted whitespace after icons #153

Closed

ashawley force-pushed the domingohui-master branch from 65aa56b to 006a676 Compare August 17, 2017 14:09

ashawley added 13 commits January 14, 2018 17:27

Use rss title in config for social link

011da96

Remove unnecessary whitespace change

2ad65f7

Remove viewBox property from .svg-icon CSS

e2fb235

Mention social links in README

93771ba

Fix paths to social links for Flickr and SO

d4a29a3

Don't escape social usernames expecting + or /

55527bc

Drop Stackoverflow since username problems

12b1365

Drop xmlns from svg tag

74f7fa5

Sync about page with template in jekyll

82a644d

Add social_sites hash

2a5bcb0

Add relative_url for link to feed.xml

609bbf8

Rename icons.svg to minima-social-icons.svg

0667a59

Add note to History.markdown

cf672b8

ashawley force-pushed the domingohui-master branch from ce30656 to cf672b8 Compare January 14, 2018 22:32

ashawley mentioned this pull request Jan 15, 2018

[Experimental] Use SVG Sprite #73

Closed

Merge branch 'master' into domingohui-master

f5065c6

DirtyF merged commit 7495baa into jekyll:master Jan 16, 2018

This was referenced Jan 16, 2018

Remove /feed.xml #182

Closed

Inline SVG v SVG files #64

Closed

AndrewJudson pushed a commit to AndrewJudson/minima that referenced this pull request Jan 27, 2018

Update social_sites in README

789a00d

This changes the documentation to match the version in _config.yml in jekyll#141.

AndrewJudson mentioned this pull request Jan 27, 2018

Update social_sites in README #189

Merged

ashawley mentioned this pull request Apr 25, 2018

Fix SVG validation error #236

Merged

ashawley mentioned this pull request May 6, 2018

Overrides On GitHub Pages For Some _includes Files Works Goofy #240

Closed

ashawley mentioned this pull request Oct 23, 2018

Add StackExchange / StackOverflow social links #291

Closed

ashawley deleted the domingohui-master branch October 29, 2018 13:34

hookex pushed a commit to hookex/red-star that referenced this pull request May 13, 2019

Merge pull request jekyll#141 from ashawley/domingohui-master

30b80d1

Refactor social links

hookex pushed a commit to hookex/red-star that referenced this pull request May 13, 2019

Update social_sites in README

7d110eb

This changes the documentation to match the version in _config.yml in jekyll#141.

jekyll locked and limited conversation to collaborators Oct 29, 2019

jekyllbot added the frozen-due-to-age label Oct 29, 2019

		@@ -0,0 +1,14 @@
		<ul class="social-media-list">
		{% if site.dribbble_username %}<li><a href="https://dribbble.com/{{ site.dribbble_username \| cgi_escape \| escape }}"><svg class="svg-icon" xmlns="http://www.w3.org/2000/svg"><use xlink:href="/assets/icons.svg#dribbble"></use></svg> <span class="username">{{ site.dribbble_username \| escape }}</span></a></li>{% endif %}

		@@ -0,0 +1,27 @@
		<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">

		<symbol id="dribbble" fill-rule="evenodd" clip-rule="evenodd" stroke-linejoin="round" stroke-miterlimit="1.414"><path d="M8 16c-4.41 0-8-3.59-8-8s3.59-8 8-8 8 3.59 8 8-3.59 8-8 8zm6.747-6.905c-.234-.074-2.115-.635-4.257-.292.894 2.456 1.258 4.456 1.328 4.872 1.533-1.037 2.624-2.68 2.93-4.58zM10.67 14.3c-.102-.6-.5-2.688-1.46-5.18l-.044.014C5.312 10.477 3.93 13.15 3.806 13.4c1.158.905 2.614 1.444 4.194 1.444.947 0 1.85-.194 2.67-.543zm-7.747-1.72c.155-.266 2.03-3.37 5.555-4.51.09-.03.18-.056.27-.08-.173-.39-.36-.778-.555-1.16-3.413 1.02-6.723.977-7.023.97l-.003.208c0 1.755.665 3.358 1.756 4.57zM1.31 6.61c.307.005 3.122.017 6.318-.832-1.132-2.012-2.353-3.705-2.533-3.952-1.912.902-3.34 2.664-3.784 4.785zM6.4 1.368c.188.253 1.43 1.943 2.548 4 2.43-.91 3.46-2.293 3.582-2.468C11.323 1.827 9.736 1.176 8 1.176c-.55 0-1.087.066-1.6.19zm6.89 2.322c-.145.194-1.29 1.662-3.816 2.694.16.325.31.656.453.99.05.117.1.235.147.352 2.274-.286 4.533.172 4.758.22-.015-1.613-.59-3.094-1.543-4.257z"/></symbol>

Refactor social links #141

Refactor social links #141

Uh oh!

Conversation

ashawley commented Jul 12, 2017

Uh oh!

ashmaroli commented Jul 13, 2017

Uh oh!

ashmaroli commented Jul 13, 2017

Uh oh!

ashawley commented Jul 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashawley commented Jul 19, 2017

Uh oh!

ashmaroli commented Jul 20, 2017

Uh oh!

ashawley commented Jul 20, 2017

Uh oh!

ashmaroli commented Jul 20, 2017

Uh oh!

ashawley commented Jul 20, 2017

Uh oh!

ashmaroli commented Jul 20, 2017

Uh oh!

ashmaroli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashawley Jul 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DirtyF Jul 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashmaroli commented Jul 21, 2017

Uh oh!

DirtyF left a comment

Choose a reason for hiding this comment

Uh oh!

DirtyF Jul 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashawley Jul 21, 2017 •

edited

Loading

DirtyF Jul 22, 2017 •

edited

Loading

DirtyF Jul 22, 2017 •

edited

Loading