RatanShreshtha / DeepThought

A simple blog theme focused on writing powered by Bulma and Zola.
https://deepthought-theme.netlify.app/
MIT License
177 stars 92 forks source link

Feature Proposal: optional JavaScript libraries #19

Closed martinmcwhorter closed 2 years ago

martinmcwhorter commented 3 years ago

I would like to make a pull request to make JavaScript libraries included a set of configuration options.

Before I make the changes and submit a PR I would like to get your feedback on whether this is something that you will likely accept and which of these two implementation options I should choose.

Within the site.js file there is some instantiation code for each library, such as galleria. If galleria is not included this needs to be handled.

We could:

  1. Make the instantiation code checking check for the presence of the library
  2. Or move the instantiation JavaScript into inline blocks in the base.html that can be conditionally included

I would choose option 2.

option 1:

// site.js
if (Galleria) {
  $(".galleria").each(function (index) {
    $(this).attr("id", `galleria-${index}`);

    var { images } = JSON.parse($(this).text());

    for (let image of images) {
      $(this).append(
        `<a href="${image.src}"><img src="${image.src}" data-title="${image.title}" data-description="${image.description}"></a>`
      );
    }

    Galleria.run(`.galleria`);
  });
}

option 2:

// base.html
<script>
{% if config.extra.javascript.galleria %}
  $(".galleria").each(function (index) {
    $(this).attr("id", `galleria-${index}`);

    var { images } = JSON.parse($(this).text());

    for (let image of images) {
      $(this).append(
        `<a href="${image.src}"><img src="${image.src}" data-title="${image.title}" data-description="${image.description}"></a>`
      );
    }

    Galleria.run(`.galleria`);
  });
{% endif %}
// more conditional javascript initializers here... 
</script>
kenohassler commented 2 years ago

@martinmcwhorter I share your opinion on the JavaScript libraries and made some changes in my fork. I chose option 1 simply because it was less of a change – but I'm not committed on this. Furthermore I replaced the JQuery code in site.js with vanilla JavaScript, making JQuery an optional dependency only needed for Galleria.js :tada:. @RatanShreshtha if you're interested, I can send a PR :smiley:

RatanShreshtha commented 2 years ago

@kenohassler Send the PR

martinmcwhorter commented 2 years ago

I went with option 2, but kept these within the template of the website. The reason being less smaller response sizes.

Like @kenohassler , I eliminated jquery for all but Galleria.

I added on a proper dark mode, not using CSS color inversion.

If any of this is wanted, I can create a PRs.

kenohassler commented 2 years ago

Hey @martinmcwhorter ,

Sounds great! I played around with a dark Bulma theme as well, we seem to have a similar taste :+1: Go ahead, send your PRs :slightly_smiling_face:

quadratecode commented 2 years ago

@martinmcwhorter would you still be willing to share your code on the dark mode improvements now that @kenohassler has kindly provided the JS update?

martinmcwhorter commented 2 years ago

I wanted to get this done sooner than later. I can work on this tomorrow, separating it out from other changes.

Should I assume we don't want any node/npm dependencies? Or is that something this project is ok with adding?

On Wed 15 Dec 2021, 09:50 quadratecode, @.***> wrote:

@martinmcwhorter https://github.com/martinmcwhorter would you still be willing to share your code on the dark mode improvements now that @kenohassler https://github.com/kenohassler has kindly provided the JS update?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RatanShreshtha/DeepThought/issues/19#issuecomment-994589748, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJDT6VPRKPRTBXQBE2Q6LDURBQF7ANCNFSM43MWGSBQ .

quadratecode commented 2 years ago

Great news! By all means there is no hurry; I just wanted to inquire given your previous comment about having added a dark mode without inversion.

Can't speak to the question about the dependencies, since @RatanShreshtha is the owner. I personally do not mind them.