Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Form submission with required field #33

Closed
tooppaaa opened this issue Dec 24, 2016 · 9 comments
Closed

Form submission with required field #33

tooppaaa opened this issue Dec 24, 2016 · 9 comments

Comments

@tooppaaa
Copy link

Hi,

When submitting a form with required empty fields, the loading class is added to the button and not removed.

Here is my implementation :

<form ng-submit="submitLogin()" promise-btn name="loginForm">
                            <md-input-container >
                                <label>Email</label>
                                <input ng-model="email" required md-no-asterisk>
                            </md-input-container>
                            <md-input-container>
                                <label>Password</label>
                                <input ng-model="password" type="password" required md-no-asterisk>
                            </md-input-container>
                            <md-button type="submit">
                                Login
                            </md-button>
                        </form>

and my controller

$scope.submitLogin = function () {
            console.debug("login submitted", $scope.loginForm)
            if($scope.loginForm.$valid)
            {
                return $auth.login($scope.account).then(function (response) { },function(error){ });
            }
        };

and my config

 var config = {
      spinnerTpl: '<div class="btn-spinner " md-colors="{background: \'primary\'}"><md-progress-circular md-mode="indeterminate" class="md-warn md-hue-3 center" md-diameter="40px"></md-progress-circular></div>',
      priority: 0,
      disableBtn: true,
      btnLoadingClass: 'is-loading',
      addClassToCurrentBtnOnly: true,
      disableCurrentBtnOnly: false,
      minDuration: 700,
      CLICK_EVENT: 'click',
      CLICK_ATTR: 'ngClick',
      SUBMIT_EVENT: 'submit',
      SUBMIT_ATTR: 'ngSubmit'
    };

The submitLogin function is only called when the form is valid and I don't want to disable it when the form is invalid.

I am happy to investigate this a bit more and create a PR but I need some guidance on where to look to make the code looking for the $valid property of the form.

Thanks
Clem

@johannesjo
Copy link
Owner

Hello there! Thanks for reporting! I made a commit which allows you to configure the button selector via the BTN_SELECTOR property of the config.

If this doesn't work: Could you by any chance provide a plunkr or something of the sort illustrating the issue?

@tooppaaa
Copy link
Author

Hello !

I applied you commit but it didn't fixed the issue.
I created the plunker : https://plnkr.co/edit/G2s27apA7h84ueviCHjI?p=preview

You just have to click "Login"
i haven't created any factory to have a proper promise as this is not needed there. If the form is valid, everything works fine.

@johannesjo
Copy link
Owner

The answer is simple: You're not giving the button any type of promise. Imho otherwise you won't need a loading spinner anyways, as the action is happening instantly. See this example: https://plnkr.co/edit/i51lgyIs9hSqhSiJ5UTG?p=preview

@johannesjo
Copy link
Owner

To quote the Readme you need to do one of the two:

// inside some controller
$scope.yourServiceCaller = function ()
{
  // needs to return a promise
  return fakeFactory.method().then(...);
};
// or 
// inside some controller
$scope.yourServiceCaller = function ()
{
  // needs to be a promise
  $scope.yourPromise = fakeFactory.method().then(...);
};

@tooppaaa
Copy link
Author

tooppaaa commented Dec 27, 2016

When the form is valid, there are no issues with a valid promise.
I did not add any as this is not needed to reproduce.

However, if you open the plunkr, hit login with an empty password, the login method is not called (you don't see the alert, the promise is not returned to the button) but the spinner is displayed.

I tried on Chrome & Edge, same behaviour.

Edit : I have a solution, let me create a PR :)

tooppaaa added a commit to tooppaaa/angular-promise-buttons that referenced this issue Dec 27, 2016
@tooppaaa
Copy link
Author

PR : @ #34

@johannesjo
Copy link
Owner

Thank you for the pr. I'm not really sure, if I want to add form validation checks with this directive though as there might be unintended side effects (usually I handle validation with ng-fab-form). My first thought would be to pass the promise to the directive directly (e.g.: <button promisie-btn=promise>) and only assign the promise/make the asynchronous call when the form is valid. I've to think about it though. I'll get back to you.

@johannesjo
Copy link
Owner

I debugged the issue a little further: This only occurs together when the option addClassToCurrentBtnOnly is set to true. You usually don't need this. Only for the rare cases when you have multiple promise-btns for the same promise, but you just want to show the spinner on the currently clicked one.

@tooppaaa
Copy link
Author

Good spot ! I guess I misunderstood the addClassToCurrentBtnOnly option.
I thought I was able to disable other buttons by specifying the same promise.

You can reject my PR, I'll set the option to false and think a bit more of how to disable the other buttons of the form while waiting for the promise.

Thanks for your help !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants