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

Improve the 'report bug' functionality to auto-fill data #33802

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

gokcekantarci
Copy link
Contributor

@gokcekantarci gokcekantarci commented Jul 12, 2024

Summary of the Pull Request

!!! Important
For testing, please change https://github.com/microsoft/PowerToys to https://github.com/gokcekantarci/PowerToys

https://github.com/microsoft/PowerToys/pull/33802/files#diff-3b008940357607ddfe1a2427962928042eb1cd920d16e17c5e76449d73b52946R123

https://github.com/microsoft/PowerToys/pull/33802/files#diff-3b008940357607ddfe1a2427962928042eb1cd920d16e17c5e76449d73b52946R123

In this PR, some fields on the link opened with the 'Report Bug' button were automatically filled. When pressed the button from both the general page and the system tray, a bug report is created on the desktop. The fields that can be filled in on the page that opens later are filled automatically.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Report_Page

General Settings
Settings_Generating
Settings_Generated

System Tray
Tray_Generating
Tray_Generated

Validation Steps Performed

General Settings

  • Click the report a bug button on the general settings page and see the generating message box.
  • Wait for the process to finish and see the message generated. Then, check the report created with the report name in the message on the desktop.
  • Press the 'OK' button and wait for the bug report link to open automatically. Check that the version field and additional information field with the OS version, .NET version, Pref language, User locale, Installation, Run as admin fields are filled in correctly.
  • Press the report a bug button again. Cancel with the Cancel button before the report is created.
  • Check that the "https://aka.ms/powerToysReportBug" link opens automatically without filling in the relevant fields. Also check that no reports are being generated on the desktop.

System Tray

  • Press the report bug button from the system tray and see the generating notification message.
  • Wait for the process to finish and see the message generated. Then, check the report created with the report name in the message on the desktop.
  • Wait for the bug report link to open automatically. Check that the version field and additional information field with the OS version, .NET version, Pref language, User locale, Installation, Run as admin fields are filled in correctly.

@htcfreek

This comment has been minimized.

@htcfreek
Copy link
Collaborator

Does this PR implements #28791? It looks like it does.

@niels9001
Copy link
Contributor

Should the "the file is created on the desktop" dialog open the desktop folder for you so you can attach it quicker? Now it requires the user to open File Explorer first, navigatw to desktop etc

@Jay-o-Way
Copy link
Collaborator

it requires the user to open File Explorer first, navigatw to desktop etc

Ehm, normally the desktop can be only one click or hotkey away.

placeholder: |
OS version
.Net version
Preferred System Language
Copy link
Collaborator

Choose a reason for hiding this comment

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

Personally, I don't like these words. "Preferred" implies the possibility that the choice is not implemented. And does it mean OS lang or App lang? (since PowerToys UI should be using the app setting, right?)
It's just vague.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to "System Language"

DefaultButton="Primary">
<StackPanel>
<TextBlock Text="Creating bug report, please wait..."
VerticalAlignment="Center"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted.

HorizontalAlignment="Center"
TextAlignment="Center"/>
<ProgressBar IsIndeterminate="True"
VerticalAlignment="Bottom"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted.

<ProgressBar IsIndeterminate="True"
VerticalAlignment="Bottom"
Height="20"
Margin="10"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Margin="10"/>
Margin="8"/>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 368 to 369
<controls:PageLink x:Uid="GeneralPage_Documentation" Text="Documentation" Link="https://aka.ms/PowerToysOverview" />
<controls:PageLink x:Uid="General_Repository" Text="Repository" Link="https://aka.ms/powertoys" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the Text property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted.

{
Title = "Error",
Content = $"An error occurred: {ex.Message}",
CloseButtonText = "OK",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Errors are not very okay 🤓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

error

I manually throw an exception and error message looks like above. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should log errors as well, not just show.

@niels9001
Copy link
Contributor

niels9001 commented Jul 14, 2024

it requires the user to open File Explorer first, navigatw to desktop etc

Ehm, normally the desktop can be only one click or hotkey away.

It might, but why inform the user to go to navigate to a certain folder (which in my case has a lot of files and I'd need to manually search for it) if we could open it and highlight the file? E.g. using: https://stackoverflow.com/questions/13680415/how-to-open-explorer-with-a-specific-file-selected

Or at the very least provide a clickable link to the folder.

This is pretty common in confirmation dialogs in Windows apps when e.g. exporting a file like a video.

@Jay-o-Way
Copy link
Collaborator

navigate to a certain folder

Well, the user can also minimize all windows to see the actual desktop (not as a folder). I think we're talking about a few second or clicks, but oh wel...
The idea to use Explorer with the file selected is actually fine. But then, why bother to ask? If a user creates the file, we can assume* they want to use it right away.

assume?

Said with a hint of sarcasm... I know assuming something is dangerous and often unwanted. But that doesn't seem to be a concern for Microsoft. Remember all the complaints about Windows 10 being installed without consent? Or importing data from Chrome into Edge. Or simply giving the user the choice between "Yes" and "Later" - where's the "No"? It's typical for Microsoft...


additionalInfo += windowsSettings + "%0a" + installScope + "%0a" + isElevatedRun;

gitHubURL = "https://github.com/microsoft/PowerToys/issues/new?assignees=&labels=Issue-Bug%2CNeeds-Triage&template=bug_report.yml" +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why assigning labels this way. They are defined in the template. Worry that nobody imagine about this piece of code later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://aka.ms/powerToysReportBug
When you open this currently working link, the link that opens is as follows. Also, it does not assign these labels when they are not added.

https://github.com/microsoft/PowerToys/issues/new?assignees=&labels=Issue-Bug%2CTriage-Needed&template=bug_report.yml&title=

@gokcekantarci
Copy link
Contributor Author

navigate to a certain folder

Well, the user can also minimize all windows to see the actual desktop (not as a folder). I think we're talking about a few second or clicks, but oh wel... The idea to use Explorer with the file selected is actually fine. But then, why bother to ask? If a user creates the file, we can assume* they want to use it right away.

assume?

I tried it as in the link suggested by @niels9001 https://stackoverflow.com/questions/13680415/how-to-open-explorer-with-a-specific-file-selected

I think opening both the link and the folder would create a bigger mess on the screen. The name of the file is specified in the last message. It will not be difficult to find this file on the desktop. So I think we need to move forward this way. If there are requests or complaints from users, we can easily add it.

@jaimecbernardo
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

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

Thanks for opening the PR. Some feedback from testing:
Gave it a test, but from tray icon, it creates the zip on Desktop, but then the following Dialog appears:
image
Doesn't make much sense having Ok and Cancel, since both seem to do the same thing.
It also should be likely an Error or Warning icon instead of the info. The message is also not too helpful.
All following attempts to generate the Bug Report after the first don't create the zip and just show the error message right away.
These changes also make the tray icon block while the Bug Report is being generated, which is a regression from 0.82.1
This needs more logs when there are errors so that we can understand what's going on when this errors occur.

@crutkas
Copy link
Member

crutkas commented Sep 10, 2024

@gokcekantarci did you resolve the feedback from @jaimecbernardo ?

This comment has been minimized.

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

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

Hi @gokcekantarci ,
Gave this another try, btu this was the page that's shown at the end:
image
This is not what's intended, can you please take a look?

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.

6 participants