search

ortiza5 / modding-vivaldi

A not-even-WIP-anymore collection of guides about modifying the Vivaldi browser
The Unlicense
7 stars 1 forks source link

Mutation observer guide - Advanced JS #16

Open ortiza5 opened 3 years ago

ortiza5 commented 3 years ago

I know this is a low priority since the more basic guides are not done yet, but I started work on an advanced guide for using mutation observers.

It might need rewriting, but I decided to make this pull request because of a forum post that this guide could help.

code3z commented 3 years ago

Nice, thanks! The good thing about advanced guides is that we can assume the user has a certain level of knowledge - so we don’t need to wait for the previous guide to be completed.

code3z commented 3 years ago

Also, I’d move the file into making-mods/advanced-js/.

Maybe not. What if we split making-mods into beginner-modding, advanced-css and advanced-js

ghost commented 3 years ago

Also, I’d move the file into making-mods/advanced-js/.

Maybe not. What if we split making-mods into beginner-modding, advanced-css and advanced-js

Still it is all just subcategories of making mods.

code3z commented 3 years ago

Still it is all just subcategories of making mods

Maybe. But if you are browsing the guide, beginner tutorials and advanced JS are completely separate. And too many sibfolders doesn’t make for easy browsing.

ghost commented 3 years ago

Still it is all just subcategories of making mods

Maybe. But if you are browsing the guide, beginner tutorials and advanced JS are completely separate.

…but if you want to know how to use mods, they are all the same for you.

And too many sibfolders doesn’t make for easy browsing

That’s why the contents should be in the main README.

code3z commented 3 years ago

And too many subfolders doesn’t make for easy browsing

That’s why the contents should be in the main README.

So, in the readme, you would want this:

Making mods

But to me, this seems cleaner (only 3 sections deep):

Using Mods

ghost commented 3 years ago

So, in the readme, you would want this:

Almost, but more like this (because it doesn’t take so much space without the headings)

code3z commented 3 years ago

Ok, I guess.

ortiza5 commented 3 years ago

get rid of that text-transform: capitalize in headings

Sorry, force of habit. Should be fixed now.

:set textwidth=80 (not for code)

This one has me a little confused.

I assume you don't mean to break paragraphs into 80 char widths. I am guessing that you were talking about the code blocks at the end of the file. Those were just pasted in as placeholders to get the early draft of the guide visible for sharing purposes, and apparently my auto-formatter doesn't apply styling to code blocks.

I plan to rewrite some parts of the code and add some short comments. The current stuff was just pulled directly from some mods I had written.

I prefer adding blank lines only around top-level lists, but decide for yourself on this.

The spacing was to allow more spacing between the list items. I looked like word soup to me without the extra whitespace. It can be changed if needed.

Also, I’d move the file into making-mods/advanced-js/.

The structure seemed a bit nebulous at the time of writing, but I agree with the proposed folder structure.

ghost commented 3 years ago

get rid of that text-transform: capitalize in headings

Sorry, force of habit. Should be fixed now.

Thanks!

:set textwidth=80 (not for code)

This one has me a little confused.

I assume you don't mean to break paragraphs into 80 char widths. I am guessing that you were talking about the code blocks at the end of the file. Those were just pasted in as placeholders to get the early draft of the guide visible for sharing purposes, and apparently my auto-formatter doesn't apply styling to code blocks.

I plan to rewrite some parts of the code and add some short comments. The current stuff was just pulled directly from some mods I had written.

I meant it like I wrote it, like it is in CONTRIBUTING:

try to keep every line short; in plain text, wrap lines at column 80

So you don’t need to worry about code so much, but about the text. (My reasoning: code usually has some short-enough widths where it is wrapped, but in plain text every paragraph would make a single, very long line.)

I prefer adding blank lines only around top-level lists, but decide for yourself on this.

The spacing was to allow more spacing between the list items. I looked like word soup to me without the extra whitespace. It can be changed if needed.

Aha, well spotted.

Also, I’d move the file into making-mods/advanced-js/.

The structure seemed a bit nebulous at the time of writing, but I agree with the proposed folder structure

Thanks!

code3z commented 3 years ago

This is looking very nice 👍

How do you select text to comment on like tiosgz does?

Maybe this would help with snapshot changes:

let footer = document.querySelector("#browser > footer");
let footerObserver = new MutationObserver(function (mutations) {
  mutations.forEach(function(mutation) {
    if (footer.querySelector(".toolbar-statusbar"))
     {
      console.log("status bar on");
    } else { console.log("status bar off or set to overlay"); }
  });
});

// Observe Child Elements of #browser > footer, using defined observer called "footerObserver"
footerObserver.observe(footer, { childList: true });

If the way I did it is not good, maybe that's something to mention in the guide. You have the explanations and stuff for the example mods as TODOs - but would you just want to add them as code comments?

// Fill in "..." with appropriate values

const targetElement = document.querySelector(" ... ");
const configurationOptions = { ... : true};

function callback(mutations) {
  mutations.forEach(function (mutation) {
    if (mutation.type === ... ) {
      // act on mutation here
    }
  }
}

const observer = new MutationObserver(callback);
observer.observe(targetElement, configurationOptions);

I would make two templates: one for detecting a subtree change and one for detecting a class change, either in addition or instead of this template. For the subtree change template, targetElement could be changed to parentElement. (I know mutation observers can be used for other things, or to do both of these things, which is why you should keep the global template, but most of the time, when modding Vivaldi, only subtree and class changes are necessary to detect.)

Also might want to briefly mention IIFE to prevent callback from being overwritten?

I really would not define configuration options separately, it does not make sense to me.

Also, would you want to use arrow functions here?

ghost commented 3 years ago

How do you select text to comment on like tiosgz does?

Go to the files changed tab, hover over the line & click the blue + at the left. When you’re done adding comments, click the green button at the top right.

I would make two templates

IMO it’s better to give working examples that people can try & play around with – my experience with templates so far has been that I usually didn’t know what to put in. Or did I not understand your intention correctly? (The usual structure at the beginning is good IMO, since it is completed with the code examples.)

Also might want to briefly mention IIFE to prevent callback from being overwritten?

Superfluous, I think. It should be in the getting started guide or in tips & tricks, but not everywhere. It is in no way related to mutation observers.

I really would not define configuration options separately, it does not make sense to me.

Depends. For the single-key it might look better to have them defined at function call, for the longer ones it’s good to define them separately.

Also, would you want to use arrow functions here?

See discussion above. As long as it’s consistent across the file (in cases where the difference doesn’t matter), I’m fine with both.

code3z commented 3 years ago

(The usual structure at the beginning is good IMO, since it is completed with the code examples.)

I definitely like the code examples, but I am saying there are really two usual structures here not one.

Superfluous, I think. It should be in the getting started guide or in tips & tricks, but not everywhere. It is in no way related to mutation observers.

Yes, but then we need to make it clear in Getting Started and Tips and Tricks that much of the code in the guide should (usually) be used inside your mod’s IIFE.

I really would not define configuration options separately, it does not make sense to me.

Depends. For the single-key it might look better to have them defined at function call, for the longer ones it’s good to define them separately

Well, according to the guide as it is now, there are really only 2 true/false configuration options, and those are only used once throughout the script. I don’t think that’s enough to define it separately.

code3z commented 3 years ago

@ortiza5 You should fetch and merge 3 commits from tiosgz:master

ghost commented 3 years ago

I don’t think it makes any sense until the PR is going to be merged very soon. Relax 😎

Plus there are no merge conflicts – it will only have to be updated for the new repo structure (& headings reverted to standard English style – sorry)