-
-
Notifications
You must be signed in to change notification settings - Fork 562
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
Move login form to a new page #2290
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume scripts/php/loginpage.php
can go as well?
login.php
Outdated
|
||
<div class="card"> | ||
<div class="card-body login-card-body"> | ||
<p class="login-box-msg">Sign in to start your session</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this text at all? If so, maybe
<p class="login-box-msg">Sign in to start your session</p> | |
<p class="login-box-msg">Sign in to access Pi-hole's dashboard</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion about this, but I would follow one of this ideas:
- remove the text;
- use "Sign in to access the dashboard"
I don't think we need to use "Pi-hole" again. The name and logo are is already above this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I would remove the text. What else should happen after you clicked "log in" on a login page other than being logged in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree.
<link rel="stylesheet" href="style/vendor/bootstrap/css/bootstrap.min.css?v=<?php echo $cacheVer; ?>"> | ||
<?php if ($auth) { ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to leave this here or move it to header_authenticated.php
? It won't be used on the login page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
This file (header.php
) is included in both cases.
This if
is needed to only use this block when authenticated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is exactly my point. header.php
is used for login.php
and header_authenthicated.php
. But this part is never used on the login.php
because $auth
is not set when the login
is shown. Therefore it makes sense to me to move it to header_authenticated
and remove the if
clause.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No.
header_authenticated.php
is used ONLY for authenticated pages.
header.php
is used in both cases (it is included in header_authenticated.php
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to put all <head>
stuff in one place to make it easy to maintain.
I'm also trying to group all <scripts>
and all <link>
tags together in just one place.
We can remove the if
s and separate these tags in 2 files, but I think its more logical to put them together.
Bumps [actions-ecosystem/action-add-labels](https://github.com/actions-ecosystem/action-add-labels) from 1.1.0 to 1.1.3. - [Release notes](https://github.com/actions-ecosystem/action-add-labels/releases) - [Commits](actions-ecosystem/action-add-labels@v1.1.0...v1.1.3) --- updated-dependencies: - dependency-name: actions-ecosystem/action-add-labels dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Co-authored-by: yubiuser <ckoenig@posteo.de> Signed-off-by: DL6ER <DL6ER@users.noreply.github.com>
Co-authored-by: yubiuser <ckoenig@posteo.de> Signed-off-by: DL6ER <DL6ER@users.noreply.github.com>
…quire* Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
…xer to Symfony Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
- Remove unnecessary "Sign in to start" text; - Adjust LCARS colors: login error message; - Fix javascript error in `footer.js`. Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
b377acf
to
8a94dfd
Compare
Superseded by #2294 |
Please make sure you
This PR creates a new login page, with its own layout.
Removing the old login page and creating a new one.
Maybe a few screenshots.
By submitting this pull request, I confirm the following:
git rebase
)