settings-level-expert: hide them by default#3487
Merged
Merged
Conversation
Contributor
Author
|
To clarify the reasoning behind this change. I should have done this in 8556b65. Because without this, when the DOM loads, the expert settings elements are visible and then become hidden. This way, they are hidden by default unless the option to show them is true. Which matches the previous behavior. |
yubiuser
reviewed
May 26, 2025
yubiuser
left a comment
Member
There was a problem hiding this comment.
I think there is no need to have settings-level-expert in settings-all.lp as the page is only accessible when expert mode is enabled.
Additionally, I think it can be removed in child elements when the parents have it already set.
| <button type="button" class="btn btn-danger confirm-flusharp btn-block button-pad destructive_action" disabled>Flush network table</button> | ||
| </div> | ||
| <div class="col-xs-12 col-md-6 col-lg-3 settings-level-expert"> | ||
| <div class="col-xs-12 col-md-6 col-lg-3 settings-level-expert d-none"> |
Member
There was a problem hiding this comment.
Suggested change
| <div class="col-xs-12 col-md-6 col-lg-3 settings-level-expert d-none"> | |
| <div class="col-xs-12 col-md-6 col-lg-3"> |
| <td id="sysinfo-dns-replies-unanswered"> </td> | ||
| </tr> | ||
| <tr class="settings-level-expert"> | ||
| <tr class="settings-level-expert d-none"> |
Member
There was a problem hiding this comment.
Suggested change
| <tr class="settings-level-expert d-none"> | |
| <tr> |
| </tbody> | ||
| </table> | ||
| <!-- <table class="table table-striped table-bordered nowrap settings-level-expert"> | ||
| <!-- <table class="table table-striped table-bordered nowrap settings-level-expert d-none"> |
Member
There was a problem hiding this comment.
Suggested change
| <!-- <table class="table table-striped table-bordered nowrap settings-level-expert d-none"> | |
| <!-- <table class="table table-striped table-bordered nowrap"> |
| </div> | ||
|
|
||
| <div class="col-sm-12 settings-level-expert save-button-container"> | ||
| <div class="col-sm-12 settings-level-expert d-none save-button-container"> |
Member
There was a problem hiding this comment.
Suggested change
| <div class="col-sm-12 settings-level-expert d-none save-button-container"> | |
| <div class="col-sm-12 save-button-container"> |
| mg.include('scripts/lua/settings_header.lp','r') | ||
| ?> | ||
| <div class="row settings-level-expert" id="advanced-content"> | ||
| <div class="row settings-level-expert d-none" id="advanced-content"> |
Member
There was a problem hiding this comment.
Suggested change
| <div class="row settings-level-expert d-none" id="advanced-content"> | |
| <div class="row" id="advanced-content"> |
Contributor
Author
|
What you are suggesting is irrelevant to my change though. I simply
restored the previous behavior. You can make your changes after this lands
if you want.
…On Mon, May 26, 2025, 22:10 yubiuser ***@***.***> wrote:
***@***.**** commented on this pull request.
I think there is no need to have settings-level-expert in settings-all.lp
as the page is only accessible when expert mode is enabled.
Additionally, I think it can be removed in child elements when the parents
have it already set.
------------------------------
In settings-system.lp
<#3487 (comment)>:
> @@ -260,7 +260,7 @@ mg.include('scripts/lua/settings_header.lp','r')
<div class="col-xs-12 col-md-6 col-lg-3">
<button type="button" class="btn btn-danger confirm-flusharp btn-block button-pad destructive_action" disabled>Flush network table</button>
</div>
- <div class="col-xs-12 col-md-6 col-lg-3 settings-level-expert">
+ <div class="col-xs-12 col-md-6 col-lg-3 settings-level-expert d-none">
⬇️ Suggested change
- <div class="col-xs-12 col-md-6 col-lg-3 settings-level-expert d-none">
+ <div class="col-xs-12 col-md-6 col-lg-3">
------------------------------
In settings-system.lp
<#3487 (comment)>:
> @@ -230,7 +230,7 @@ mg.include('scripts/lua/settings_header.lp','r')
<th scope="row">Unanswered queries:</th>
<td id="sysinfo-dns-replies-unanswered"> </td>
</tr>
- <tr class="settings-level-expert">
+ <tr class="settings-level-expert d-none">
⬇️ Suggested change
- <tr class="settings-level-expert d-none">
+ <tr>
------------------------------
In settings-system.lp
<#3487 (comment)>:
> @@ -185,7 +185,7 @@ mg.include('scripts/lua/settings_header.lp','r')
</tr>
</tbody>
</table>
-<!-- <table class="table table-striped table-bordered nowrap settings-level-expert">
+<!-- <table class="table table-striped table-bordered nowrap settings-level-expert d-none">
⬇️ Suggested change
-<!-- <table class="table table-striped table-bordered nowrap settings-level-expert d-none">
+<!-- <table class="table table-striped table-bordered nowrap">
------------------------------
In settings-all.lp
<#3487 (comment)>:
> @@ -28,7 +28,7 @@ mg.include('scripts/lua/settings_header.lp','r')
<!-- dynamically filled with content -->
</div>
- <div class="col-sm-12 settings-level-expert save-button-container">
+ <div class="col-sm-12 settings-level-expert d-none save-button-container">
⬇️ Suggested change
- <div class="col-sm-12 settings-level-expert d-none save-button-container">
+ <div class="col-sm-12 save-button-container">
------------------------------
In settings-all.lp
<#3487 (comment)>:
> @@ -13,7 +13,7 @@ mg.include('scripts/lua/header_authenticated.lp','r')
PageTitle = "All Settings"
mg.include('scripts/lua/settings_header.lp','r')
?>
-<div class="row settings-level-expert" id="advanced-content">
+<div class="row settings-level-expert d-none" id="advanced-content">
⬇️ Suggested change
-<div class="row settings-level-expert d-none" id="advanced-content">
+<div class="row" id="advanced-content">
—
Reply to this email directly, view it on GitHub
<#3487 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACVLNJEXLPOVNCZ7VCUUFL3ANRKVAVCNFSM6AAAAAB54WPBHSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNRYHE4DINZUGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Matches the previous behavior before 8556b65 Signed-off-by: XhmikosR <xhmikosr@gmail.com>
Contributor
|
Conflicts have been resolved. |
PromoFaux
approved these changes
May 30, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Matches the previous behavior before 8556b65
Thank you for your contribution to the Pi-hole Community!
Please read the comments below to help us consider your Pull Request.
We are all volunteers and completing the process outlined will help us review your commits quicker.
Please make sure you
What does this PR aim to accomplish?:
How does this PR accomplish the above?:
Link documentation PRs if any are needed to support this PR:
By submitting this pull request, I confirm the following:
git rebase)