Login Button Not Working with Keyboard Enter

[Ghislain Hachey]

One must click Login with mouse.

[Ghislain Hachey]

OK. I've got a issue60 branch with several small improvements to the login page. I'll wait for the PR to be merged so I can pull latest develop into it before pushing and making a pull request.

[Ghislain Hachey]

OK. I've pulled your latest develop into my issue60 local branch and resolved conflicts and now ready to submit. It seems clean with my new login page small improvements. How should I do this? Just like last time pushing to my miemis? and then PR? Feels awkward. See #59 for remaining of this discussion. And if you can find a little time to get this one out of the way I would appreciate that.

[Brian Lewis]

How about - push your branch to origin, then create the pull request directly from the branch?

When accepted the pull request will merge to develop, then you can pull develop back into miemis to preserve your customised web config.

[Ghislain Hachey]

feat(login): Login Page Enhancements

• change ng-click to ng-submit to get automatic Enter key press working (and not just having to click on login with mouse)
• add required validation on username (email) and password fields
• add basic validation check something is entered in both fields
• only enable Log In button when two fields have the required data
• disable Log In button when authentication is in process
• Show spinning icon when authentication is in process

Closes #60

→ <<cset 655fcddffbc2>>

[Ghislain Hachey]

feat(authenticationController.ts): improve login page

• accepts enter keypress for login (and not just mouse click)
• add loading icon to show user the system is responding and login in

Closes #60

→ <<cset 212b28e7b1f3>>

[Ghislain Hachey]

Yes, thanks I did that. I'm still a bit rusty with my squashed commits. I'll refine that.

[Ghislain Hachey]

feat(authenticationController.ts): improve login page

• accepts enter keypress for login (and not just mouse click)
• add loading icon to show user the system is responding and login in
• login failed now styled in red and button returns to enable

Closes #60

→ <<cset 6fa1e4c7c68c>>

[Ghislain Hachey]

I fixed the login failed issue. I'm still having lots of difficulty trying to rebase and squash my commits into a single clean commit with clear message :(

[Ghislain Hachey]

feat(authenticationController): Login Page Enhancements

• change ng-click to ng-submit to get automatic Enter key press working (and not just having to click on login with mouse)
• add required validation on username (email) and password fields
• add basic validation check something is entered in both fields
• only enable Log In button when two fields have the required data
• disable Log In button when authentication is in process
• Show spinning icon when authentication is in process
• login failed message style in red and re-enable login button

Closes #60

→ <<cset 449fc675d0b6>>

[Ghislain Hachey]

Forget issue60 branch. I've put my changes to issue60-2. I got too messed up with rebases and squashing and spent the day fighting with that and power brownout, argh. This is likely not the last time I get messed up in Git :)

[Brian Lewis]

I'm no git-fu guru, but perhaps there is a simple solution here. I think the point is that the "squashing" doesn't happen when you push your branch to origin (ie the server), it happens when the pull request from that branch is merged into develop.

So e.g.

• you create a new branch locally, using source tree;
• when you are ready, you push that branch to origin. You push every commit - the local and origin branches are (and stay) the same
• you create a pull request from that branch (in source tree or at bitbucket)

When the pull request is merged, on bitbucket, you get the standard merge commit format - a heading to say it is a merge, and the title of the pull request becomes the description of the single commit applied to develop.

In other words, the default behaviour here is to use a merge commit rather than a fast-forward (which would simply advance the head of the develop branch to the head of the pulled branch)

This article seems to describe what we want to do.

So I think the question becomes: what do we want to put in the description of the merge commit that is generated from the Pull Request? As noted, by default it is the merge header and the title of the PR; let's remove the merge header and I think we have what we want. :

The merge commit is annotated to show it closes the source branch - and the source branch contains all the detailed commit history if you ever need to forensically audit that. (We could by convention prune these branches after e.g. 2 release cycles?)

I like the feat(authentication) format you have used; derived from the type of the issue, and the "module" of the application, followed by the description.

Looking through the angular example, I'd propose we try to keep an "imperative" form for this description, starting with a verb;

e.g.

Enhance login page behaviour

not

Login page enhancements

and not past tense:

Fixed login page

So we'd end up with something like this:

[Brian Lewis]

feat(authenticationController): Login Page Enhancements

• change ng-click to ng-submit to get automatic Enter key press working (and not just having to click on login with mouse)
• add required validation on username (email) and password fields
• add basic validation check something is entered in both fields
• only enable Log In button when two fields have the required data
• disable Log In button when authentication is in process
• Show spinning icon when authentication is in process
• login failed message style in red and re-enable login button

Closes #60

→ <<cset 449fc675d0b6>>

[Ghislain Hachey]

How do you "the default behaviour here is to use a merge commit rather than a fast-forward"? I see the default can be change through a config file, do you have that in Bitbucket?

The way I was doing it is some habit taken from development workflow of Angular and angular projects. The difference are further discussed here.

I think your suggested approach is good for most our cases (short lived branches to work on issues, features, etc). But the rebase approach is more versatible and might be needed to. I'd like to get it to work as well. But I like I said I am prompt credentials when trying to squash my remote commits locally.

Yes regarding imperative form for message.

I'm not so sure about closing source branch on merge. What if it needs a bit more work before the merge is accepted? How will that work?

[Ghislain Hachey]

And here is pretty well put. Now it's pretty clear in my head. They will inevitably become both important I think. Of course, the a nitty-gritty details to learn about each but...

Merge Let's say you have created a branch for the purpose of developing a single feature. When you want to bring those changes back to master, you probably want merge (you don't care about maintaining all of the interim commits).

Rebase A second scenario would be if you started doing some development and then another developer made an unrelated change. You probably want to pull and then rebase to base your changes from the current version from the repo.

[Ghislain Hachey]

I guess we should probably have these discussion in #59 :)

[Ghislain Hachey]

Another important concept to consider here. Personally, I would not want to keep all the micro commits of branches for auditing (even if grouped and hidden in main branch). I don't think it would be useful and would just keep too much "change cruft" if that makes sense. I think better to keep chunks of work small in nature (well as small as possible) but recorded as single commits in the grand scheme of things. Does that make sense?

[Ghislain Hachey]

Regarding git merge --squash and git merge --no-ff --no-commit: In other words, I don't think its important to know the details of what went into the head of the developer (whether he modified the HTML before the typescript, etc), but better to see the big picture of the change in a single place for auditing (all three changed files for a new feature X).