User management Frontend #23
Conversation
How can I link the issue? |
|
The first comment of this PR should include the text 'fix #11'... and than check the 'Linked issues' section on the right. |
templates/login.html
Outdated
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/4.4.1/css/bootstrap.min.css"> | ||
| <link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Advent+Pro"> | ||
| <link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Fira+Sans+Extra+Condensed"> | ||
| <link rel="stylesheet" href="assets/css/styles.css"> |
There was a problem hiding this comment.
Where is this file? and why are the styles defined inline for each element and not in it with classes?
There was a problem hiding this comment.
Was this comment addressed?
templates/assets/js/main.js
Outdated
| document.getElementById("main_block_btn").onclick = signinBtn; | ||
|
|
||
| $("#btn_signup").click(function() { | ||
| document.getElementById("main_block_btn").onclick = signupBtn; |
There was a problem hiding this comment.
Why are you inconsistent and sometimes use $(id) and sometimes getElementById(id) ?
There was a problem hiding this comment.
I'm waiting for @eidoil32 response.
templates/assets/js/main.js
Outdated
| '<div class="w-100 rounded-right" style="background-color: rgba(0,0,0,0.7);padding-top: 0;padding-bottom: 0;"></div>' + | ||
| '</div>'; | ||
|
|
||
| function ajaxCall(url, data, method, successCallback, errorCallback) { |
There was a problem hiding this comment.
Why did you wrap the ajax call ?
There was a problem hiding this comment.
For my opinion It's more legible to manage the JS Ajax call in one function other than copy the same code (like the dataType) all over the application.
There was a problem hiding this comment.
Wrapping the ajax call limits its functionallity.
If you would want to send an additional info with the ajax call, you will need to change the function signature and fix its call everywhere you used it.
This problem actually taking effect here because you should add contentType to the list of parameters ajax is taking.
The default contentType is "application/x-www-form-urlencoding", and we will need to send "application/json" as well.
There was a problem hiding this comment.
I'm waiting for @eidoil32 response.
There was a problem hiding this comment.
Hi @lioramilbaum, this PR need to be closed due to inactivity.
PR #52 takes care of this issue now :)
Added login / signup page
Screenshot:
@timshik
fix #11