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 more information to DevDiagnostics resource usage page #3755

Merged
merged 5 commits into from
Sep 5, 2024

Conversation

xhan65
Copy link
Contributor

@xhan65 xhan65 commented Sep 4, 2024

Summary of the pull request

Extend DevDiagnostics's resource usage page to have both system resource usage information and Application resource information.

  • Add a System tab and Application tab to resource usage page so user can switch between tabs to get detailed information. Application tab only appears when DevDiagnostics is attached to a process.
  • In System tab, add detailed hardware and sensor information using LibreHardwareMonitorLib APIs. The hardware and sensor information are visualized via an expandable Treeview.

{C6D38E97-AA8E-4E7F-9542-C4A343EE4A05}
{879499AC-E241-49D5-817A-60AA108CFA66}

References and relevant issues

#3296

result = CommonHelper.GetLocalizedString("ConnectionSpeedSensorValue1Gbps");
break;
default:
if (_sensor.Value < 1024)
Copy link
Contributor

@andreww-msft andreww-msft Sep 4, 2024

Choose a reason for hiding this comment

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

Perhaps define all these magic numbers as consts somewhere? #Resolved

<TreeViewItem AutomationProperties.Name="{x:Bind Name}">
<Grid>
<Grid.ColumnDefinitions>
<ColumnDefinition Width="200"/>
Copy link
Contributor

@andreww-msft andreww-msft Sep 4, 2024

Choose a reason for hiding this comment

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

Is this always going to be big enough, if the user has text size set to >100%? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. No it won't be big enough if the text size gets bigger. Is there a recommended way to set the width? I noticed that with text size increased, the column for current "cpu usage", "memory usage" is not big enough either

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, we could adjust the width dynamically using UISettings.TextScaleFactor as a multiplier - but that seems overkill. Alternatively, set TextWrapping=wrap, but that might look odd. Simplest would be to simply pick a much bigger width (say 300?), as we have lots of horizontal real estate to play with - not perfect, but likely would cover most cases.

@andreww-msft
Copy link
Contributor

andreww-msft commented Sep 4, 2024

Please can you clarify:

  1. When are the sensor values updated - all the time, or only when the (System/Application) tab is open? or only when a specific item in the tree is actually visible?
  2. How frequently are they updated?
    ... can we get a summary of the behavior added to the tooltip for the page header? #Resolved

@jaholme
Copy link
Contributor

jaholme commented Sep 4, 2024

Have you validated that the resource usage is read by narrator, as well as the tabsd at the top? #Resolved

UpdateSensorData(node);
}
}
}
Copy link
Contributor Author

@xhan65 xhan65 Sep 4, 2024

Choose a reason for hiding this comment

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

Stale code, will clean #Resolved

@xhan65
Copy link
Contributor Author

xhan65 commented Sep 4, 2024

  1. Sensor values are updated when the user is in Resource usage page (both system and application tab). When user navigated away from that page, the timer is stopped.
  2. Sensor values are updated every 2 seconds.

Added tooltip information to page header.


In reply to: 2327983675

@xhan65
Copy link
Contributor Author

xhan65 commented Sep 4, 2024

Narrator recognizes resource usage and treeview, but it wasn't able to read the tabs. I filed this bug to track the tab header accessibility issue.: #3766 I'll fix it in the next PR.


In reply to: 2329804070

@jaholme
Copy link
Contributor

jaholme commented Sep 4, 2024

it might be as simple as adding an x:uid and something like the following to the resources.resw


In reply to: 2330275851

@xhan65
Copy link
Contributor Author

xhan65 commented Sep 5, 2024

Thanks! I was having trouble having narrator read the header text block, but after adding uid and AutomationProperties in resource.resw for the tabitem it worked. I'll close the bug.


In reply to: 2330307374

Copy link
Contributor

@jaholme jaholme left a comment

Choose a reason for hiding this comment

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

:shipit:

@xhan65 xhan65 merged commit e260332 into main Sep 5, 2024
4 checks passed
jaholme added a commit that referenced this pull request Sep 6, 2024
jaholme added a commit that referenced this pull request Sep 6, 2024
…3755)" (#3797)

This reverts commit e260332.

Co-authored-by: Jason Holmes <27746781+jaholme@users.noreply.github.com>
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