epidrome / cover-card

:necktie: Online business card with a minimal landing page theme for any social media or online presence account: fork, edit, and go!
https://epidrome.github.io/cover-card/
MIT License
108 stars 109 forks source link

Social links management #5

Closed epidrome closed 5 years ago

epidrome commented 6 years ago
vivekkrish commented 5 years ago

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:

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 commented 5 years ago

@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 #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 commented 5 years ago

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 commented 5 years ago

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 https://github.com/epidrome/cover-card/pull/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 commented 5 years ago

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 commented 5 years ago

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.