Added manual control possibility over holy loader

[lorikku]

Description

I've had a few instances where I needed to trigger the holy loader manually (eg. because of an async operation), but I couldn't because the possibility wasn't there. So I added two new functions (startHolyProgress and stopHolyProgress) to control the holy loader manually. Simple yet very useful.

Concretely what was done:

1. Added usage of React.useRef because using let is not best practice. It's important to keep the same reference to the HolyLoader object, using a let does not guarantee this. A ref is the correct method. (let actually caused a bug when I was trying to stopProgress which would fire but holyLoader.complete() didn't do anything!)
2. Added startHolyProgress and stopHolyProgress to control the holy loader manually using custom events, meaning you can only use these in Client components!
3. Fixed small typo (sppeed -> speed)

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue) -> I guess kind of with React.useRef usage
• New feature (non-breaking change which adds functionality)

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass with my changes

[lorikku]

Reviewed my own code, lgtm!

[lorikku]

@tomcru writing a test necessary here? Would require to set up React component testing, also dependency injection to get holyLoader.status and check if it equals (1) when it should etc.

Otherwise ready to be reviewed and merged!

[tomcru]

This looks exciting and I've had something similar in mind - will test it out asap!

[lorikku]

This looks exciting and I've had something similar in mind - will test it out asap!

So apparently it's the good ol' "it used to work but now it doesn't" scenario.. I just tested rn and it's not working anymore, will have to see what's wrong and update it here.

Bugs to fix:

• Does NOT work on Server components (won't work)
• Is not firing anymore for some reason whatsoever?

I'll get it sorted out soon, for now will put this in draft.

[lorikku]

Okay so I realised that this won't ever work on the server side because I use document.dispatchEvent which requires document which is not available on the server.

Anything requiring UI interactivity will forever be restricted to Client components only (which makes sense obv). That means that this functionality will too.

I marked it in the documentation to make sure people know. But it definitely works now on client side now!

@tomcru ready to review now!

[tomcru]

I was thinking the same, that'd you'd only ever use the trigger funcs to start/stop the loader in client components anyways 👍

Will review either today or tomorrow latest, thank you!

[tomcru]

Looks and works great!
I would only suggest renaming start/stopHolyProgress to start/stopHolyLoader - because the HolyProgress component is not something users interact with. I think it'll be easier to remember the naming.

[tomcru]

I noticed a little edge case where you can theoretically dispatch a ton of events which messes with the behaviour of the progress bar (I'm frantically clicking the Start button in this video)
https://github.com/tomcru/holy-loader/assets/35841182/de825998-45f6-420f-9fa7-52fba7ac01c7

Compared to the normal behaviour:
https://github.com/tomcru/holy-loader/assets/35841182/bbc9fa8e-3ffa-4520-aa4f-b6711837f067

[lorikku]

I noticed a little edge case where you can theoretically dispatch a ton of events which messes with the behaviour of the progress bar (I'm frantically clicking the Start button in this video) https://github.com/tomcru/holy-loader/assets/35841182/de825998-45f6-420f-9fa7-52fba7ac01c7

Compared to the normal behaviour: https://github.com/tomcru/holy-loader/assets/35841182/bbc9fa8e-3ffa-4520-aa4f-b6711837f067

I think this might to be an issue related to the startTrickle function, which creates a setTimeout every time it's fired, without checking if any other is in progress already. I'll try to fix it now.

[lorikku]

@tomcru I fixed the issue by adding a incrTimeout variable which gets cleared when startTrickle is called.

Before:

After:

[tomcru]

Maybe a simpler solution would be to update the HolyProgress start() method from:

  public start = (): HolyProgress => {
    if (this.status === null) {
      this.setTo(0);
    }
    
    this.startTrickle();

      if (this.settings.showSpinner === true) {
        this.createSpinner();
      }

    return this;
  };

to:

public start = (): HolyProgress => {
    if (this.status === null) {
      this.setTo(0);

      this.startTrickle();

      if (this.settings.showSpinner === true) {
        this.createSpinner();
      }
    }

    return this;
  };

As a result, incrementing is only started if it's a new progress bar - and not if it is already running.

This works on my side, can you possibly confirm this also works in your use case?

[lorikku]

@tomcru works like a charm! Pushed the solution right now as well :)

[tomcru]

Thank you! good stuff 🔥

[lorikku]

@tomcru been testing this yesterday throughout my platform, it works insanely well. Very happy with the results and good UX addition.