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

Add links to dashboard boxes #2386

Merged
merged 3 commits into from Oct 8, 2022
Merged

Add links to dashboard boxes #2386

merged 3 commits into from Oct 8, 2022

Conversation

rdwebdesign
Copy link
Member

@rdwebdesign rdwebdesign commented Oct 7, 2022

What does this PR aim to accomplish?

  • Add links to the 4 boxes on the top of dashboard;
  • Rearrange the boxes colors.

How does this PR accomplish the above?

Adding the following links:

  • Number of active clients (on Total queries box) ➔ link to network.php;
  • Blocked queries box ➔ link to queries.php?forwarddest=blocked (blocked queries);
  • Percentage box ➔ link to queries.php (all queries);
  • Domains on Adlists box ➔ link to groups-adlists.php;

What documentation changes (if any) are needed to support this PR?

none


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

  • I have read the above and my PR is ready for review. Check this box to confirm

@rdwebdesign rdwebdesign changed the title Topboxlink Add links to dashboard boxes Oct 7, 2022
@rdwebdesign rdwebdesign requested a review from a team October 7, 2022 05:51
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

I like what I see. Three remarks:

  1. There is a tooltip on "Total queries" and "Domains on adlist". This pops up when hovering over the new link. I think it should pop up when hovering over the boxes (old behavior) not the link.
  2. I would rename "XX clients" to "XX active clients" because the number is only the number of active clients in the last 24h. It's confusing otherwise, because the network table might display way more clients.
  3. I would change the link for "Percentage Blocked" to the query log in general, not only the blocked queries. As a percentage needs a reference value, displaying only the denominator is not very meaningful.

@rdwebdesign
Copy link
Member Author

rdwebdesign commented Oct 7, 2022

I don't agree with item 1:
the tooltip IS on the box. The link triggers the tooltip because it's part of the box.

Pihole_graph_colors


Edit:
OK. Changed: no more tooltips over the links (the rest of the box shows the tooltip).

@rdwebdesign rdwebdesign force-pushed the topboxlink branch 2 times, most recently from c3457e7 to a77cec8 Compare October 7, 2022 17:53
@rdwebdesign rdwebdesign requested review from yubiuser and a team October 7, 2022 17:55
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>
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.

None yet

2 participants