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 ad guard home #937

Merged
merged 5 commits into from May 20, 2023
Merged

✨ Add ad guard home #937

merged 5 commits into from May 20, 2023

Conversation

manuel-rw
Copy link
Collaborator

@manuel-rw manuel-rw commented May 16, 2023

πŸ€– Generated by Copilot at 2b5a051

Summary

πŸ›‘οΈπŸ§ͺπŸ“Š

This pull request adds support for AdGuard Home integration in the DNS hole module, which allows users to control and monitor their AdGuard Home apps from the dashboard. It also improves the image rendering of the integration icons and the accuracy of the DNS queries metric. The pull request introduces new files for the AdGuard SDK, Zod schemas, and types, and refactors some existing files to use shared utility functions and classes.

Sing, O Muse, of the valiant coder who dared to tame the AdGuard Home
And with skillful schemas and fetches, he made the DNS hole module his own
He trimmed the hostnames with trimStringEnding, a handy function of strings
And he showed the queries with formatNumber, a graceful touch of decimals

Walkthrough

  • Add support for AdGuard Home integration as a new DNS hole option (link, link, link, link, link, link, link, link, link, link, link, link)
  • Replace native img elements with Image component from @mantine/core for better styling and performance in IntegrationSelector component (link, link, link)
  • Fix pihole image URL in IntegrationSelector component to match correct file name (link)
  • Use trimStringEnding function from src/tools/shared/strings.ts to trim hostname in PiHoleClient and AdGuard classes (link, link, link, link, link)
  • Disable no-await-in-loop ESLint rule for src/pages/api/modules/dns-hole/summary.ts file, as the loop needs to await API calls for each app (link)
  • Change Consola.error message in src/pages/api/modules/dns-hole/summary.ts file to use generic term "DNS hole" instead of "PiHole" (link)
  • Change formatNumber function call in DnsHoleSummaryWidgetTile component to use 3 decimal places instead of 0 for DNS queries today metric (link)

@vercel
Copy link

vercel bot commented May 16, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
homarr βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback May 20, 2023 0:41am

@manuel-rw manuel-rw marked this pull request as draft May 16, 2023 19:32
@manuel-rw manuel-rw changed the title Widget/add ad guard home ✨ Add ad guard home May 16, 2023
@manuel-rw manuel-rw linked an issue May 16, 2023 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented May 16, 2023

πŸ“¦ Next.js Bundle Analysis for homarr

This analysis was generated by the Next.js Bundle Analysis action. πŸ€–

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 366.14 KB (🟑 +4.94 KB)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Four Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load
/ 78.65 KB (🟑 +3.31 KB) 444.79 KB
/[slug] 78.66 KB (🟑 +3.31 KB) 444.8 KB
/login 1.15 KB (🟑 +70 B) 367.29 KB
/migrate 8.14 KB (🟑 +436 B) 374.28 KB
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

@manuel-rw
Copy link
Collaborator Author

manuel-rw commented May 16, 2023

  • WAITING FOR CONFIRMATION FROM USER

@manuel-rw manuel-rw self-assigned this May 16, 2023
ajnart
ajnart previously approved these changes May 18, 2023
Copy link
Owner

@ajnart ajnart left a comment

Choose a reason for hiding this comment

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

Just annoyed about the new files. I think we already have way more than enough folders and files in the project. Imo all the tools could be in one ts file

src/tools/shared/strings.ts Outdated Show resolved Hide resolved
src/tools/server/sdk/adGuard/adGuard.type.ts Outdated Show resolved Hide resolved
src/pages/api/modules/dns-hole/summary.ts Show resolved Hide resolved
src/pages/api/modules/dns-hole/summary.ts Show resolved Hide resolved
@ajnart
Copy link
Owner

ajnart commented May 18, 2023

@manuel-rw don't forget to change the status from draft when the PR is ready

@manuel-rw manuel-rw merged commit fb52c4b into dev May 20, 2023
5 of 6 checks passed
@manuel-rw manuel-rw deleted the widget/add-ad-guard-home branch May 20, 2023 12:42
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.

AdGuard Home integration
3 participants