Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

login-signin - Added fixes according to review #52

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Asaf-Federman
Copy link
Collaborator

fixes #11
Cloned previous PR #23 due to inactivity
Decoupled the css from the login template
Fixed inconsistency with get-element-id
Fixed file structure according to the existing one

edison/static/css/login-signin.css Outdated Show resolved Hide resolved
edison/static/css/login-signin.css Outdated Show resolved Hide resolved
edison/static/css/login-signin.css Outdated Show resolved Hide resolved
edison/static/js/login-signup.js Outdated Show resolved Hide resolved
edison/static/js/login-signup.js Outdated Show resolved Hide resolved
edison/static/js/login-signup.js Outdated Show resolved Hide resolved
edison/static/js/login-signup.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@DoRTaL94 DoRTaL94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -0,0 +1,106 @@
<!DOCTYPE html>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be better to extend the existing 'layout.html' instead of creating a completely different one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ill look into that when I have the time, kinda busy studying for exams at the moment

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

Successfully merging this pull request may close these issues.

User management Frontend
4 participants