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

Visual fixes and improvements #711

Merged
merged 7 commits into from
Dec 2, 2024

Conversation

Malex14
Copy link
Contributor

@Malex14 Malex14 commented Nov 17, 2024

This PR fixes a lot of small CSS-Issues and improves the cpu and memory charts.

Charts

I've updated the chart library to the latest version and added a tooltip that displays the cpu / ram usage at the point of time where the cursor hovers over the chart. Additionally the cpu chart and legend show the usage now in percent (instead of 0 to cpu count) and has its y max fixed at 100%. I've also added a hover text to the cpu legend that displays the averages of the past 1, 5 and 15 minutes. Also the y-Axis is now readable in dark mode:

Click to show screenshots

Original:

1
10

Improved:

2
3
6

Overflowing Boxes

To fix the mobile overflows, I mostly just reenabled the commented out @media rules. But a few other changes had to be made too:

Click to show screenshots

Original:

11
12
13

Improved:

4
5
7
8

Overlapping Text

Lastly there was some overlapping text in the "External monitoring tool" section:

Click to show screenshots

Original:

14

Improved:

9

@Malex14 Malex14 force-pushed the visual-fixes-and-improvements branch 2 times, most recently from cfb0ada to 54d3785 Compare November 18, 2024 10:34
@Malex14
Copy link
Contributor Author

Malex14 commented Nov 18, 2024

I just rebased the pr on top of the current master branch

@Malex14
Copy link
Contributor Author

Malex14 commented Nov 24, 2024

Hello @kesselb, since you reviewed other PRs in the past, can you (or maybe someone else) please take a look at this PR?

@kesselb
Copy link
Collaborator

kesselb commented Nov 25, 2024

Hi,

Thank you so much for your pull request—it’s much appreciated!

Could you please update the implementation of Linux.getCpuCount to use /proc/cpuinfo, similar to how we handle it in Linux.getCpuName? I’d like to avoid executing shell commands where possible to keep things consistent and efficient.

Let me know if you have any questions or need help with this!

Signed-off-by: Malex14 <39774812+Malex14@users.noreply.github.com>
Signed-off-by: Malex14 <39774812+Malex14@users.noreply.github.com>
Changes in detail:
- show load in percent
- pined chart y max to 100%
- added units to chart y-Axis
- added small (by default) blue box to CPU load legend (like the one that was in the RAM legend)
- added hover text to the "Load average: XX.X % (X.XX) last minute" that displays the load averages for 1, 5 and 15 minutes
- added tooltip to both charts that display the cpu load / ram usage at the time where the cursor hovers.

Signed-off-by: Malex14 <39774812+Malex14@users.noreply.github.com>
…commented out `@media` css rules, removed remains from when the css-file was scss

Signed-off-by: Malex14 <39774812+Malex14@users.noreply.github.com>
Signed-off-by: Malex14 <39774812+Malex14@users.noreply.github.com>
@Malex14 Malex14 force-pushed the visual-fixes-and-improvements branch from 54d3785 to 1d234f3 Compare November 25, 2024 19:29
Signed-off-by: Malex14 <39774812+Malex14@users.noreply.github.com>
@Malex14 Malex14 force-pushed the visual-fixes-and-improvements branch from 1d234f3 to c04c3a5 Compare November 25, 2024 19:30
@Malex14
Copy link
Contributor Author

Malex14 commented Nov 25, 2024

Thank you for the quick response. I've changed the function as you asked.

Signed-off-by: Malex14 <39774812+Malex14@users.noreply.github.com>
@joshtrichards
Copy link
Member

Linked two Issues this likely closes.

Copy link
Contributor

github-actions bot commented Dec 1, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

$numCpu = -1;

try {
$numCpu = intval($this->executeCommand('sysctl -n hw.ncpu')); //TODO: this should be tested if it actually works on FreeBSD
Copy link
Collaborator

Choose a reason for hiding this comment

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

That yields the expected result for my cpu.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok 👍 , then I will remove the comment

@kesselb
Copy link
Collaborator

kesselb commented Dec 2, 2024

/backport to stable30

@Malex14
Copy link
Contributor Author

Malex14 commented Dec 2, 2024

should I rebase the commits on top of master again?

@kesselb kesselb merged commit 9f913b3 into nextcloud:master Dec 2, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: mobile view of settings > system out of boxes Infobox should be inline-block
3 participants