fbrettnich / whmcs-supportpin-module

SupportPin WHMCS Addon | Let your customers generate a support/phone pin to identify your customers faster, for example on the phone
GNU General Public License v3.0
25 stars 12 forks source link

Added rotating Icon #6

Closed bastianleicht closed 3 years ago

bastianleicht commented 3 years ago

Changes

Closes Issue: NaN

Description

This Commit adds some CSS, that rotate the Pin regenerate Icon when hovered.

fbrettnich commented 3 years ago

Hi, thanks for your PR I think an animation when hovering makes little sense and is unnecessary. The animation would make more sense if the button is clicked until the new support PIN is there. PR #3 will improve PIN regeneration using Ajax, this could well be added there..

bastianleicht commented 3 years ago

Hi, thanks for your Reply. I think this is a good idea. I'm going to upate this PR, when #3 is merged.

fbrettnich commented 3 years ago

Hi, thanks for your Reply. I think this is a good idea. I'm going to upate this PR, when #3 is merged.

FYI: PR #3 has been merged

bastianleicht commented 3 years ago

Alright, I'm going to update this PR in the next week.

fbrettnich commented 3 years ago

Hi, your new commits implement rotation on hover again. The rotation should only start when clicking (before Ajax requests) and stop rotating after the new PIN is displayed (when Ajax is finished).

bastianleicht commented 3 years ago

Yes, I just updated my codespace. The next push is going to implement that feature.

bastianleicht commented 3 years ago

It now rotates the Icon while the Pin is generated.

fbrettnich commented 3 years ago

@ConanDoyl could you please submit a review?

ConanDoyl commented 3 years ago

@ConanDoyl could you please submit a review?

Yes i'll check these changes and reply again if i'm done.

ConanDoyl commented 3 years ago

Seems good, but i would like to change one more thing to make it smoother. Let me show:

Current Script:

<script>
    let pinIcon = document.getElementById("pinIcon");
    function Request(url, callback){
        $.ajax({
           type: "POST",
           crossDomain: false,
           url: url,
           data: { "PIN": true },
           success: function(respond){
                callback(respond);
           },
           error: function(response){
               console.log(response.status);
           }
       });
    };

    function RenewPIN(){
        pinIcon.classList.add("icon-rotate");
        Request("index.php?m=supportpin&page=renew", function(response){
            $("#sPIN").html(response.PIN);
            pinIcon.classList.remove("icon-rotate");
        })
    }
    </script>

Let us update it to this one, i'll explain later why.


<script>
var isLoading = false;

function Request(url, callback, errorCallback = false) {
    $.ajax({
        type: "POST",
        crossDomain: false,
        url: url,
        data: { "PIN": true },
        success: function (respond) {
            callback(respond);
        },
        error: function (response) {
            if (!errorCallback) {
                console.log(response.status);
                return;
            }
            errorCallback(response);
        }
    });
};

function RenewPIN() {
    if (isLoading){
        return; // Break to avoid generating multiple Pins 
    }
    isLoading = true;
    $('#pinIcon').addClass('icon-rotate');
    Request("index.php?m=supportpin&page=renew", function (response) {
        $("#sPIN").html(response.PIN);
        $('#pinIcon').removeClass('icon-rotate');
        isLoading = false; // Remove the loading state again, if we add the button a ID we can update it also on Loadin with the attribute disabled and later on removing it again
    }, function (reponse) {
        // On Failure, we need to remove the rotating animation or if we also change the button, we need to remove this tho.
        isLoading = false;
        $('#pinIcon').removeClass('icon-rotate');
    });
}
    </script>

We using jQuery because WHMCS is using it also we do not need to afraid using it too. jQuery makes some things easier. Of course its not required. The second thing is, i've added is a loading State to check if the button has been pressed to avoid multiple post calls to the endpoint because it's absolutely not necessary. Then i've added a errorCallback to handle the failure event, if the request fails we change the rotating icon again back to static and make the user able to click it again.