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 Info_dashboard package #7424

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

Conversation

tp82d1
Copy link

@tp82d1 tp82d1 commented Nov 25, 2024

This package (luci-app-tplink_dashboard) introduces a dashboard interface for OpenWrt, inspired by the user interface of TP-Link firmware. The primary goal is to simulate the visual and functional aspects of TP-Link dashboards within the OpenWrt ecosystem.

Features

  • TP-Link-Inspired Design: A minimalist and visually appealing design that mirrors the layout and simplicity of TP-Link firmware dashboards.
  • Dynamic Icons: Includes interactive icons for Internet connection, device detail and connected devices.
    • Internet Status: Displays the current connection status dynamically.

    • Device Detail: Displays information about the router.

    • DHCP Monitoring: Displays the number of active DHCP clients in real time.

Testing

The package was tested under the following conditions:

  • OpenWrt Versions:
    • 23.05.0

Screenshots

InternetStatus_NotConnected dashboard InternetStatus_connected DeviceInfo DHCP

@tp82d1 tp82d1 marked this pull request as draft November 25, 2024 11:04
@tp82d1 tp82d1 marked this pull request as ready for review November 25, 2024 11:13
@systemcrash
Copy link
Contributor

Have you taken those image assets from a TP-Link firmware?

@tp82d1
Copy link
Author

tp82d1 commented Nov 25, 2024

yes

@systemcrash
Copy link
Contributor

Of course have written permission from them to use them, right?

@tp82d1
Copy link
Author

tp82d1 commented Nov 25, 2024

No unfortunately, do I need to change them? if so, should I close this pull and make new one pull request? I'm rookie in GitHub.

@stangri
Copy link
Member

stangri commented Nov 25, 2024

First of all, I want to say it's a tremendous work to have a skin written for luci. If/when the IP/copyright issues of the assets and the name in the package title are resolved, I have the following comments on this PR:

  1. you're including both the applications/luci-mod-tplink_dashboard and luci-mod-tplink_dashboard directories in your PR. I'm guessing it's an oversight and you only need one of them.
  2. the proper location for modules (luci-mod-*) is actually modules, so I believe it should be modules/luci-mod-tplink_dashboard.
  3. The iptables is not a default package for 23.05, 24.10 nor snapshots and you declare exec access to iptables/ip6tables binaries. I believe the mod should support nft natively before the PR is merged.

@dannil
Copy link
Contributor

dannil commented Nov 25, 2024

No unfortunately, do I need to change them? if so, should I close this pull and make new one pull request? I'm rookie in GitHub.

There's such a thing called copyright, you can't just take assets without permission, including them in LuCI would make the project eventually liable for any legal consequences. You don't need to close the PR to update the assets, just keep pushing to the same Git branch with your updates.

@tp82d1
Copy link
Author

tp82d1 commented Nov 25, 2024

Thanks for your replies, ill try my best to solve what you have mentioned in your replies. if I couldn't push them in same branch, i'll close and reopen new one :).

@systemcrash
Copy link
Contributor

Thanks for your replies, ill try my best to solve what you have mentioned in your replies. if I couldn't push them in same branch, i'll close and reopen new one :).

Please don't - figure out how to use git. That's what it's designed for.

@tp82d1
Copy link
Author

tp82d1 commented Nov 25, 2024

Okay thanks

@systemcrash
Copy link
Contributor

Go grab some royalty-free vector graphics to replace the assets here. And credit/attribute any author involved.

@stangri
Copy link
Member

stangri commented Nov 25, 2024

Go grab some royalty-free vector graphics to replace the assets here. And credit/attribute any author involved.

It's worth checking if TP-Link may have published those assets with the GPL code.

Including their brand name in the name of the package might infringe on their copyright tho.

@tp82d1
Copy link
Author

tp82d1 commented Nov 27, 2024

Screenshots for the new update

dashboard InternetStatus_NotConnected InternetStatus_connected DeviceInfo DHCP

@KA2107
Copy link

KA2107 commented Nov 27, 2024

Is this related to #4185 ?

@tp82d1
Copy link
Author

tp82d1 commented Nov 28, 2024

No

@tp82d1 tp82d1 changed the title Add tplink_dashboard package Add Inof_dashboard package Nov 28, 2024
@tp82d1 tp82d1 changed the title Add Inof_dashboard package Add Info_dashboard package Nov 28, 2024
@systemcrash
Copy link
Contributor

systemcrash commented Nov 28, 2024

Looks considerably better.

  • can you replace the ❌ and ✅ with SVG versions? (and other assets also for that matter)
  • fix the test formalities, commit messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

This file needs to be renamed to match luci-mod-info_dashboard.

Copy link
Contributor

Choose a reason for hiding this comment

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

This file needs to be renamed to match luci-mod-info_dashboard.

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.

5 participants