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

Enhancement: resources network widget #4327

Merged
merged 13 commits into from
Nov 25, 2024
Merged

Conversation

ramazansancar
Copy link
Contributor

@ramazansancar ramazansancar commented Nov 25, 2024

upd: Docs updated for network usage.
add: Widget resorce iconChildren param added.
upd: config file default options added.
upd: resources units type added.

Proposed change

Closes #3421 (Discussion)

Type of change

  • New service widget
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation only
  • Other (please explain)

Checklist:

  • If applicable, I have added corresponding documentation changes.
  • If applicable, I have reviewed the feature and / or service widget guidelines.
  • I have checked that all code style checks pass using pre-commit hooks and linting checks.
  • If applicable, I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.

upd: Docs updated for network usage.
add: Widget resorce iconChildren param added.
upd: config file default options added.
upd: resources units type added.
Copy link
Collaborator

@shamoon shamoon left a comment

Choose a reason for hiding this comment

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

Thanks. This does need some work. I noted some small things on my phone but I need to take a closer look. It also won’t pass lint at the moment.

docs/widgets/info/resources.md Outdated Show resolved Hide resolved
src/components/widgets/resources/network.jsx Outdated Show resolved Hide resolved
src/pages/api/widgets/resources.js Outdated Show resolved Hide resolved
@shamoon shamoon changed the title feat: Added active and total for network usage (dw,up) resources. Enhancement: resources network widget Nov 25, 2024
@ramazansancar
Copy link
Contributor Author

Thanks. This does need some work. I noted some small things on my phone but I need to take a closer look. It also won’t pass lint at the moment.

I will look at the lint issue again. I have provided answers for other change requests.

As an additional feature, I have in mind to separate live network usage and total network usage, i.e. define it separately from the config, what is your opinion on this? (Those who want only one can open it from the config as they wish. Those who want both can add it by writing 2 lines in the config.)

Copy link
Collaborator

@shamoon shamoon left a comment

Choose a reason for hiding this comment

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

You can take a look my changes, this is cleaner already.

Screenshot 2024-11-24 at 8 30 18 PM

But I'm still not happy with the aesthetics.

  1. This is the only widget item with two lines by default, which is a problem.
  2. Yes, we are not going to show both of them. I'd suggest we make the throughput and totals each their own line and the widget defaults to throughput and the totals is in the 'expanded' version.

@ramazansancar
Copy link
Contributor Author

You can take a look my changes, this is cleaner already.

Screenshot 2024-11-24 at 8 30 18 PM

But I'm still not happy with the aesthetics.

  1. This is the only widget item with two lines by default, which is a problem.
  2. Yes, we are not going to show both of them. I'd suggest we make the throughput and totals each their own line and the widget defaults to throughput and the totals is in the 'expanded' version.

Thanks for the improvements. I will optimize it by writing 2 lines (Speed ​​and total) in the config as I first suggested, I will need some time for this.

@shamoon
Copy link
Collaborator

shamoon commented Nov 25, 2024

That's not going to work, and will make it overly-complicated. Take a look a the recent changes.

Screenshot 2024-11-24 at 8 41 05 PM Screenshot 2024-11-24 at 8 40 22 PM

The final issue here is the percentage bar, which you've just set... to 0?

@ramazansancar
Copy link
Contributor Author

That's not going to work, and will make it overly-complicated. Take a look a the recent changes.

Screenshot 2024-11-24 at 8 41 05 PM Screenshot 2024-11-24 at 8 40 22 PM

The final issue here is the percentage bar, which you've just set... to 0?

The most logical thing to do at the moment seems to be to keep it at 0. We can't do anything because we won't have any max data.

@shamoon shamoon enabled auto-merge (squash) November 25, 2024 06:29
Copy link
Collaborator

@shamoon shamoon left a comment

Choose a reason for hiding this comment

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

Screenshot 2024-11-24 at 10 30 23 PM

@shamoon shamoon merged commit e96b3c5 into gethomepage:dev Nov 25, 2024
4 of 5 checks passed
shamoon added a commit that referenced this pull request Nov 25, 2024
shamoon added a commit that referenced this pull request Nov 25, 2024
@o2e
Copy link

o2e commented Nov 26, 2024

I would like to add a tip, if you want to get the host's NIC information, then the docker container may need to be modified to network_mode: host. If you want to modify the port in host network mode, you also need to add environment: - PORT=3000(3000 is your port)

@o2e
Copy link

o2e commented Nov 26, 2024

I would like to add a tip, if you want to get the host's NIC information, then the docker container may need to be modified to network_mode: host. If you want to modify the port in host network mode, you also need to add environment: - PORT=3000(3000 is your port)

@ramazansancar I think it would be a good idea to add this note to the documentation, otherwise some newbies might not use it, and at least some of my friends don't know the difference between docker's network modes(including me)

@shamoon
Copy link
Collaborator

shamoon commented Nov 26, 2024

The problem with doing that is it will almost certainly break existing integrations with other containers. It was always my beef with this widget but then again the same can apply to other resources in some situations

@o2e
Copy link

o2e commented Nov 26, 2024

Yes, it's a matter of trade-offs. Despite his slight flaws, it's worth it to me because I don't have any other easy and intuitive way to see what's going on with the network. homepage serves as the homepage of my browser, the monitor window of my portable screen, and having a small, sophisticated display of network information is great for me, and I can live with a few tweaks for that (I know that Glances has a similar feature, but I don't really like it! )
image

@ramazansancar
Copy link
Contributor Author

I would like to add a tip, if you want to get the host's NIC information, then the docker container may need to be modified to network_mode: host. If you want to modify the port in host network mode, you also need to add environment: - PORT=3000(3000 is your port)

@ramazansancar I think it would be a good idea to add this note to the documentation, otherwise some newbies might not use it, and at least some of my friends don't know the difference between docker's network modes(including me)

Hi @o2e , Since I started the project with pm2 in source mode, I unfortunately have no idea what effect it will have on Docker and Kubernetes 🥲

@ramazansancar
Copy link
Contributor Author

I forgot to thank you. Thanks for supporting the development and pushing it forward at some point. It was a nice feature to the project. @shamoon

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.

3 participants