decorateCookedElement for each page change?

I’m trying to use the Mediaelement.js library to override the custom audio onebox, and I’ve got it working when the page is loaded/refreshed, however, when I navigate from one topic to another, it doesn’t seem to reload, showing instead the native Discourse audio onebox.

When I load/refresh:

If I then navigate to another topic:

I’m pretty sure I’m doing something wrong with the loading of mejs but thought maybe not and that I have to do something fancy with the onPageChange or something.

<head>

<script type="text/discourse-plugin" version="0.8.42">

let loadScript = require('discourse/lib/load-script').default;

    api.decorateCookedElement(
      element => {
        const audioplayers = element.querySelectorAll('audio');
        // console.log("player: " + audioplayers[0]);
        if (Object.entries(audioplayers).length > 0) {
            // console.log("audioplayers has length");
            loadScript(
                `https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement.min.js`
            );
            loadScript(
                `https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js`
            );
        }

        audioplayers.forEach(function (el) {
            el.classList.add("mejs__player");
            const controls = settings.theme_uploads.mejs-controls;
            // console.log("controls: " + controls);
            el.setAttribute("data-mejsoptions",
                `{
                    "pluginPath": "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/",
                    "iconSprite": "https://friends.jimkleiber.com/uploads/default/original/1X/a17d9708a19654d9155dd9b79a79a05dea580067.svg",
                    "alwaysShowControls": "true",
                    "features": ["playpause", "current", "progress", "duration", "volume"]
                }`
            );
            el.setAttribute("preload", "auto");            
        });            
      },
      { id: "mediaelement-js", onlyStream: true}
    );
</script>

css

@import "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelementplayer.min.css";

image

Does anyone have any suggestions on how I might tweak it so it automatically reloads when each page is loaded?

This is going to be a slightly long post since you posted in #dev. However, it’s very general, and it’s more about explaining the patterns to use in Discourse regardless of the library you want to integrate or the post elements you wish to target. It just happens to use the script you picked as the main example.

So, MediaElement gives you a couple of ways to initialize a new audio player.

  1. You can add a CSS class and a few attributes to the element, and it would then handle that for you automatically

  2. You manually initialize it

You’re currently using #1 so let’s look at that for a bit. When a script gives you “automatic initialization on an element,” it’s usually a quality of life improvement that the script author adds. In the script, they usually listen to the document load event, and they do some work on DOM elements that have the class/attributes that they tell you to add.

Ok, so why is it not working? As you saw, it works fine on the initial page load but doesn’t on subsequent in-app navigation. What gives?

The short answer is that Discourse is a single-page application. Elements like the <HTML> and <body> tags are sent once. So in a sense, the document only loads once. So when you navigate after the initial page loads, there are no more native “load” events that get sent. Remember, the document already loaded on the initial page view. Everything that happens after that is handled by Discourse.

Of course, that doesn’t mean that there are no events that fire on subsequent navigation. However, those are Discourse-specific events. So, third-party script authors would have no way of knowing those ahead of time. Imagine being a script author and having to cater to 100’s of different platforms? No good, yeah?

So, we can’t use the quality of life way the script author so nicely added. What next? Well, remember that we can still manually initialize the script on the target elements. So, let’s try to do that.

Earlier I mentioned that there’s only one native (browser-level) load event, but a platform like Discourse would not function well if it didn’t have its own system of events. For example, the plugin API has a method that allows you to fire scripts on virtual page navigation.

Should you use that method? No. That method is very helpful for stuff like analytics and so on. There’s no point firing a script that only handles <audio> tags on every page - especially if the page has none of those.

So, what’s next? Well, the good news is that you’ve already figured it out. decorateCookedElement is the correct method to use here.

It gives you a way to… wait for it… decorate posts :tada:

Discourse guarantees that any decorator you add will apply to every post.

Well, you’re loading the script in a post decorator, so it should be added, and it should work. How come it doesn’t on subsequent navigation?

For this, you have to understand how loadScript() works. Your code already checks if there are valid target elements before loading the script, so :+1:

However, imagine a situation when you have 20 - 30 posts in a row where they all have valid elements. Would it make sense to load the script 20 - 30 times? Obviously not.

loadScript() is smart enough to detect if the script has already been loaded. It won’t load duplicates, and it also won’t reload a script if it’s already finished downloading. You can see that here.

fullUrl above is the URL you pass to loadScript() when you call it, just like in your example.

So, now that we know this. We can kind of see why it doesn’t work on subsequent navigation.

You visit topic-a > it has an audio element >  loadScript() loads the script > the script does the fancy "auto init" thing > the script initialises on your elements > you get custom audio elements

then...

you visit topic-b > it has audio elements > loadScript() sees the script is already loaded > no fancy "auto init" > you get the default audio elements > sadness ensues

So, how do you fix this? Well, that’s what #2 from earlier is for

  1. You can add a CSS class and a few attributes to the element, and it would then handle that for you automatically

  2. You manually initialize it

So, let’s do that. It’s already documented on the page you shared. We need to call this on our target element like so

// You can use either a string for the player ID (i.e., `player`),
// or `document.querySelector()` for any selector
var player = new MediaElementPlayer("player", {
  // ... options
});

Your code already handles each individual audio element separately :+1: so we just need to modify this

audioplayers.forEach(function (el) {
  el.classList.add("mejs__player");
  const controls = settings.theme_uploads.mejs - controls;
  // console.log("controls: " + controls);
  el.setAttribute(
    "data-mejsoptions",
    `{
                    "pluginPath": "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/",
                    "iconSprite": "https://friends.jimkleiber.com/uploads/default/original/1X/a17d9708a19654d9155dd9b79a79a05dea580067.svg",
                    "alwaysShowControls": "true",
                    "features": ["playpause", "current", "progress", "duration", "volume"]
                }`
  );
  el.setAttribute("preload", "auto");
});

to this

audioplayers.forEach(function (el) {
-- el.classList.add("mejs__player");
-- const controls = settings.theme_uploads.mejs - controls;
-- // console.log("controls: " + controls);
-- el.setAttribute(
--   "data-mejsoptions",
--   `{
--                     "pluginPath": "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/",
--                     "iconSprite": "https://friends.jimkleiber.com/uploads/default/original/1X/a17d9708a19654d9155dd9b79a79a05dea580067.svg",
--                     "alwaysShowControls": "true",
--                     "features": ["playpause", "current", "progress", "duration", "volume"]
--                 }`
-- );
-- el.setAttribute("preload", "auto");
++ 
++ new MediaElementPlayer(el, {
++  // ... options
++ });
});

and then we take the options that we used to add as an attribute and pass them as an object like so

audioplayers.forEach(function (el) {
 new MediaElement(el, {
++ pluginPath: "//cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/",
++ iconSprite: settings.theme_uploads["mejs-controls"],
++ alwaysShowControls: "true",
++ features: ["playpause", "current", "progress", "duration", "volume"]
 });
});

Let’s put this on hold for now and look at the rest of the decorator. Here’s what we have so far

let loadScript = require("discourse/lib/load-script").default;

api.decorateCookedElement(
  element => {
    const audioplayers = element.querySelectorAll("audio");
    // console.log("player: " + audioplayers[0]);
    if (Object.entries(audioplayers).length > 0) {
      // console.log("audioplayers has length");
      loadScript(
        `https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement.min.js`
      );
      loadScript(
        `https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js`
      );
    }

    // forEach used to be here
  },
  { id: "mediaelement-js", onlyStream: true }
);

If you notice, you’re calling loadScript() on two different scripts. I’m not sure if this is intentional, but you only need one of those. Think of it as a full bundle and lightweight one. You want the custom audio player. So, you need the full bundle. Let’s remove the other one.

let loadScript = require("discourse/lib/load-script").default;

api.decorateCookedElement(
  element => {
    const audioplayers = element.querySelectorAll("audio");
    // console.log("player: " + audioplayers[0]);
    if (Object.entries(audioplayers).length > 0) {
      // console.log("audioplayers has length");
--    loadScript(
--      `https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement.min.js`
--    );
      loadScript(
        `https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js`
      );
    }

    // forEach used to be here
  },
  { id: "mediaelement-js", onlyStream: true }
);

You’re checking if there’s audio players in the post and conditionally load the script based on that. This can be simplified like so. First, check for the length directly.

let loadScript = require("discourse/lib/load-script").default;

api.decorateCookedElement(
  element => {
    const audioplayers = element.querySelectorAll("audio");
    // console.log("player: " + audioplayers[0]);
--    if (Object.entries(audioplayers).length > 0) {
++    if (audioplayers.length) {
      // console.log("audioplayers has length");

      loadScript(
        `https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js`
      );
    }

    // forEach used to be here
  },
  { id: "mediaelement-js", onlyStream: true }
);

then move that to the top and just return if the length is falsy (length < 0). I also removed the comments in the code

let loadScript = require("discourse/lib/load-script").default;

api.decorateCookedElement(
  element => {
    const audioplayers = element.querySelectorAll("audio");

++  if (!audioplayers.length) {
++   return;
++  }

    loadScript(
      `https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js`
    );

    // forEach used to be here
  },
  { id: "mediaelement-js", onlyStream: true }
);

Since the script’s src won’t ever change, let’s move that to a const. loadScript() is also always the same. Let’s make it a const as well.

++ const loadScript = require("discourse/lib/load-script").default;

++ const MEDIA_ELEMENT_SRC =
++  "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js";

api.decorateCookedElement(
  element => {
    const audioplayers = element.querySelectorAll("audio");

    if (!audioplayers.length) {
      return;
    }

++  loadScript(MEDIA_ELEMENT_SRC);

    // forEach used to be here
  },
  { id: "mediaelement-js", onlyStream: true }
);

and let’s put this one on hold as well. Before we can continue, we need to talk about how loadScript() works for a bit more.

If you want to use some part of a script, you want to ensure that it’s loaded before you do any work, no? Well, loadScript() handles that for you. It returns a Promise. Promises sound scary at first they’re really simple. A Promise is literally that… a promise.

You want to do some work… you promise the browser that you’ll let it know when the work is done… the browser waits for you. It’s really as simple as that. The rest is just understanding the syntax.

I won’t spend much time on that because it’s a bit out of scope for this topic.

Let’s continue. loadScript() is promise-based. Discourse promises the browser to let it know when the script has fully loaded - whether the script doesn’t exist and it has to be loaded or simply to check if it already was loaded.

So, if we do something like this

const loadScript = require("discourse/lib/load-script").default;

const MEDIA_ELEMENT_SRC =
  "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js";

api.decorateCookedElement(
  element => {
    const audioplayers = element.querySelectorAll("audio");

    if (!audioplayers.length) {
      return;
    }

++  loadScript(MEDIA_ELEMENT_SRC).then(() => {
++    // this will ONLY fire if the script has/is loaded
++    console.log("my script has loaded");
++  });

    // forEach used to be here
  },
  { id: "mediaelement-js", onlyStream: true }
);

So, now we can go back to our forEach loop from earlier and add it right in there, and we would know for sure that the script will be available.

const loadScript = require("discourse/lib/load-script").default;

const MEDIA_ELEMENT_SRC =
  "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js";

api.decorateCookedElement(
  element => {
    const audioplayers = element.querySelectorAll("audio");

    if (!audioplayers.length) {
      return;
    }

    loadScript(MEDIA_ELEMENT_SRC).then(() => {
++    audioplayers.forEach(function (el) {
++      new MediaElementPlayer(el, {
++        pluginPath: "//cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/",
++        iconSprite: settings.theme_uploads["mejs-controls"],
++        alwaysShowControls: "true",
++        features: ["playpause", "current", "progress", "duration", "volume"]
++      });
++    });
    });
  },
  { id: "mediaelement-js", onlyStream: true }
);

Now, look at the snippet above and see if anything bothers you…

The options we pass to the script instance are always the same, but we don’t have them in a const. Let’s fix that.

    const loadScript = require("discourse/lib/load-script").default;
    
    const MEDIA_ELEMENT_SRC =
      "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js";
    
++  const MEDIA_ELEMENT_CONFIG = {
++    pluginPath: "//cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/",
++    iconSprite: settings.theme_uploads["mejs-controls"],
++    alwaysShowControls: "true",
++    features: ["playpause", "current", "progress", "duration", "volume"]
++  };
    
    api.decorateCookedElement(
      element => {
        const audioplayers = element.querySelectorAll("audio");
    
        if (!audioplayers.length) {
          return;
        }
    
        loadScript(MEDIA_ELEMENT_SRC).then(() => {
          audioplayers.forEach(function (el) {
++          new MediaElementPlayer(el, MEDIA_ELEMENT_CONFIG);
          });
        });
      },
      { id: "mediaelement-js", onlyStream: true }
    );

and that’s about it. You were very close in your code; you just needed to understand loadScript() a bit more.

Now, we put all of that together.

common > header tab

<script type="text/discourse-plugin" version="0.8.42">
  const loadScript = require("discourse/lib/load-script").default;

  const MEDIA_ELEMENT_SRC =
    "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelement-and-player.min.js";

  const MEDIA_ELEMENT_CONFIG = {
    pluginPath: "//cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/",
    iconSprite: settings.theme_uploads["mejs-controls"],
    alwaysShowControls: "true",
    features: ["playpause", "current", "progress", "duration", "volume"]
  };

  api.decorateCookedElement(
    element => {
      const audioplayers = element.querySelectorAll("audio");

      if (!audioplayers.length) {
        return;
      }

      loadScript(MEDIA_ELEMENT_SRC).then(() => {
        audioplayers.forEach(function (el) {
          new MediaElementPlayer(el, MEDIA_ELEMENT_CONFIG);
        });
      });
    },
    { id: "mediaelement-js", onlyStream: true }
  );
</script>

and then a little bit of CSS to load the scripts CSS and prevent jitter while the script swaps the elements

common > css

@import "https://cdnjs.cloudflare.com/ajax/libs/mediaelement/5.0.4/mediaelementplayer.min.css";

.cooked {
  --audio-player-height: 40px;

  .mejs__container,
  audio {
    // match Media-Element.js player height to prevent jitter
    height: var(--audio-player-height);
    display: block;
  }
}

You should now see the custom player on every post that has valid elements.

With that out of the way, you should note that the library you picked is quite old. It’s transpiled for ancient browsers, and it tries to polyfill a lot of features that have since become standard.

If you know why you want to use it, that’s fine. However, if you’re just using it to customize the way the player looks, I recommend that you avoid it. I haven’t checked, but there are probably much lighter modern alternatives.

The best thing about all of this is that the implementation doesn’t change from the above. No matter what elements you want to target and what scripts you want to use. The same pattern applies. The only thing that changes is initializing the custom script. Every decent library has pretty good documentation that will guide you through that. Then, you just put that in the pattern above.

9 Likes

It’s at moments like these that I wish Meta were using Discourse Reactions - Beyond Likes, because a heart doesn’t feel sufficient for the love and sheer gratitude that I feel right now.

I was hoping someone would at least just give me a tip and yet you took the time to write down a very thorough explanation of how I arrived at where I did and walked me through how to make it work, teaching me a lot along the way (even with the little details about jitter!)

I’m learning how the structure of Discourse may help to encourage such behavior, by meaning that if I answer it well once, others can see it and I won’t have to answer it again—which encourages me to continue to build communities with this; and yet, I don’t think that fully explains why you wrote this out for me and your willingness to do so may encourage me to use this platform even more.

Thank you.


Regarding Mediaelement, yeah, it’s old, but it pairs well with the Wordpress site I have and have customized it a lot over there, trying to provide a familiar look to the user (and also not trying to learn yet another library at the moment :smiley:)

4 Likes

A follow up and maybe a dumb question: I’m trying to load multiple scripts now, as the Mediaelement allows plugin scripts. I want to make sure all the scripts load before returning the promise.

I’ve tried to do it through looping through the constants of the script sources and then creating an array of promises, then using Promise.all() for the initializing of the players, and yet when I do, I’m getting an error saying that mejs is not found, which I believe is the namespace or something for calling different functions within the mediaelement-and-player.

      let scripts_loaded = [];
      
      MEDIA_ELEMENT_SCRIPTS.forEach(function (src, index){
          scripts_loaded[index] = loadScript(src);
      });
      
      Promise.all(scripts_loaded).then(() => {
            audioplayers.forEach(function (el) {
              new MediaElementPlayer(el, MEDIA_ELEMENT_CONFIG);
            });
      });

However, if I chain them manually, it seems to work, as such:

      loadScript(MEDIA_ELEMENT_SRC).then(() => {
          loadScript(MEDIA_ELEMENT_SPEED_SRC).then(() => {
            loadScript(MEDIA_ELEMENT_SKIP_BACK_SRC).then(() => {
                loadScript(MEDIA_ELEMENT_JUMP_FORWARD_SRC).then(() => {
                    audioplayers.forEach(function (el) {
                        new MediaElementPlayer(el, MEDIA_ELEMENT_CONFIG);
                    });
                });
            });
        });
      });

In this instance, I’m only using a handful of scripts, so I just manually typed them all in, I was just curious if I’m missing something obvious about the Promise.all() function, or if there is a Discourse function that allows me to load multiple scripts from an array.

1 Like

Your code should work fine. You’ve just stumbled unto a bug in Discourse.

loadScript() doesn’t set the async attribute for the scripts it loads. So, it defaults to async="true" and that misses up your load order. It’s a browser quirk. You have to force async="false" for scripts loaded with JS.

Plugins are smaller, so they load faster than the main bundle, but them being async means they no longer respect the load order - wait for the main bundle to load and execute before executing.

It probably went unnoticed because loadScript is not nested anywhere in core, as far as I know. You would normally bundle files that need to work together. So, to answer your other question. No, there are no Discourse functions that handle that kind of thing.

Your other snippet should work as well. To make it a little bit easier to read, maybe try chaining them without nesting them.

// This goes outside the decorator
const PLUGINS = {
  speed: "https://example.com/foo.js",
  skipBack: "https://example.com/bar.js",
  jumpForward: "https://example.com/baz.js"
};

// Then do your work inside the decorator
loadScript(MEDIA_ELEMENT_SRC)
  .then(() => loadScript(PLUGINS.speed))
  .then(() => loadScript(PLUGINS.skipBack))
  .then(() => loadScript(PLUGINS.jumpForward))
  .then(() => {
    audioplayers.forEach(function (el) {
      new MediaElementPlayer(el, MEDIA_ELEMENT_CONFIG);
    });
  });
4 Likes

Oh goodness, thank you for letting me know!

Ok, sounds good.

Wow, I appreciate this so much. Much much cleaner. Thank you again :pray:

1 Like