Skip to content
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

Social links management #5

Closed
4 tasks done
epidrome opened this issue May 9, 2018 · 6 comments
Closed
4 tasks done

Social links management #5

epidrome opened this issue May 9, 2018 · 6 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed PR welcome

Comments

@epidrome
Copy link
Owner

epidrome commented May 9, 2018

@vivekkrish
Copy link
Contributor

vivekkrish commented Jan 20, 2019

Hi @epidrome -

Thanks for this minimalistic theme. Definitely agree that a more flexible system for social links management would make this theme even more awesome!

Based on this, I have tried to implement a system that makes use of Jekyll Data Files (https://jekyllrb.com/docs/datafiles/) to be accessed via the page templating system. This allows the use of Yaml, Json or Csv format files that contain structured data and are made available via a dictionary based data-structure to the template.

Here is my implementation:

  • gh-pages: Addition of a _data/social.yml asset and a social-order setting to the _config.yml file: gh-pages...vivekkrish:gh-pages
  • master: Refactoring the _includes/social.html to use Liquid templating system to loop through sites defined in the _config.yml file, get the URLs, icon CSS labels from _data/social.yml and organize them according to the desired order: master...vivekkrish:master

You can see an example implementation of this system on my demo instance: https://vivekkrish.com/cover-card

Look forward to your review and thoughts! If you feel like this is a good way forward, I can clean things up and issue PRs to your repo with some documentation on how to make use of this new system.

Thank you!

@epidrome
Copy link
Owner Author

@vivekkrish thank you, your solution is very good!

Before we move to the PRs, there are some comments from the point of view of simplicity for developers and users:

  1. it seems that we get rid of _include/too_social.html file 🙌 but we get a new file in the _data folder 😱 . is it possible to have all data in the main _config.yml file, which, after all, is a mandatory configuration file by the jekyll system? by the way, this change might eliminate the parameter of the include social.html at the default layout and might also make simpler the code of the social file. At the same time the users will have to edit just one configuration file even if they want to add custom links.

  2. the new data structure for social links has many options that are not clear to me. is there any significant benefit to have two parameters for url, why not just one as suggested in added social networks #7 ? I can think of a situation where a social site changes the url scheme for social profiles, but how often does this happen?

  3. it is obvious that these changes will break the sites of current users, but i am willing to take this risk now that the theme is in beta. I am happy to see my own site to break, so that I will follow the current documentation in the readme to point to an older version. This way i will be able to update the documentation so that it reflects your changes and provides a migration path for current users.

I am confident that your suggested changes will move the theme to version 0.4 and I am looking forward to your comments and/or pull request!

@vivekkrish
Copy link
Contributor

vivekkrish commented Feb 1, 2019

Hi @epidrome,

Thanks for the review and comments regarding my changes. Glad that you like it!

  1. Great suggestion, I initially implemented a _data/social.yml file due to the availability of the built in Jekyll functionality to use data files as configuration (https://jekyllrb.com/docs/datafiles/). However, it should be possible to move those configs to _config.yml as you mention. Based on this recommendation, I have made further changes:

  2. Here is a recap of the social links templating via config (https://github.com/vivekkrish/cover-card/blob/bf27ba1c86d5c143161bb65d87f9ad904a77e2ca/_config.yml#L44-L62). For example, different types of sites can be configured based on their intricacies:

    • Twitter, has a simple URL pattern twitter.com/USERNAME, and is served via https by default, and the font-awesome icon-class is fab fa-2x fa-twitter. As such, the config will be like so:
      - name: twitter
        url-pattern: twitter.com/__USERNAME__
    
    • Facebook, is very similar to twitter in terms of URL pattern (i.e. username following the site URL) and scheme, however the font-awesome icon class is different (fab fa-2x fa-facebook-f). As such, it's config is modified accordingly:
      - name: facebook
        url-pattern: facebook.com/__USERNAME__
        icon-class: fab fa-2x fa-facebook-f
    
    • Tumblr, sometimes uses a URL pattern where the username prefixes the domain, such as USERNAME.tumblr.com. As such, the config can be modified to suit the needs:
    - name: tumblr
      url-pattern: __USERNAME__.tumblr.com
    
    • Finally, something like email has a totally different pattern and protocol/scheme (i.e. mailto: instead of https://), and the font-awesome icon class is envelope (which doesn't match e-mail). As such, the configuration is set like so:
    - name: email
       url-scheme: 'mailto:'
       url-pattern: '__USERNAME__'
       icon-class: fas fa-2x fa-envelope
    

In all of these cases, the placeholder __USERNAME__ is replaced by the profile names set by the users in the earlier part of _config.yml. Does this make sense?

Keeping the above configuration separate from what the user will modify to set their site username ensures that the system is cleaner (which goes back to my initial decision to use Jekyll datafiles).

  1. I agree! That is a great way to test out the functionality and eventually document for others. There could be a blog post or detailed README which explains the config and setup.

Thanks again for this wonderful theme! Look forward to making these changes into an official release.

@epidrome
Copy link
Owner Author

epidrome commented Feb 1, 2019

thanks @vivekkrish it looks great now!

  1. do we really need the jekyll-admin gem? feel free to make a PR!
  2. you are right! one minor change, the site.github variable seems to be reserved by the github pages system, so we should not use the github variable name, see github to github-user #6
  3. once your PR is merged, i will revise the README and add the following note to the new release:

users have two options 1. switch the remote-theme setting to a previous commit 2. create a new config file

@vivekkrish
Copy link
Contributor

vivekkrish commented Feb 1, 2019

Hi @epidrome,

  1. The jekyll-admin gem adds functionality to allow users to modify the site configuration from the admin console (see docs here: https://jekyll.github.io/jekyll-admin/) instead of the command-line. You can test it out for yourself like so:

    $ git clone https://github.com/vivekkrish/cover-card
    $ cd cover-card
    $ bundle install
    $ bundle exec jekyll serve
    

    Then navigate to http://127.0.0.1:4000/admin
    Do let me know what you think! Based on that, you can decide to keep/remove the plugin.

  2. Sounds good, I will make the change to the variable name and the corresponding icon-class configuration, since we can no longer use the name to construct the font-awesome icon class.

  3. Will make a PR very soon, once you've had a chance to review ^1.

Thank you!

@vivekkrish
Copy link
Contributor

I've opened 2 PRs #20 and #21 for your review.
I've gone ahead and removed the jekyll-admin functionality, it can be added back later if needed.

@epidrome epidrome added the good first issue Good for newcomers label May 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed PR welcome
Projects
None yet
Development

No branches or pull requests

2 participants