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

Update peek.md #5168

Open
wants to merge 7 commits into
base: docs
Choose a base branch
from
Open

Update peek.md #5168

wants to merge 7 commits into from

Conversation

daverayment
Copy link

Added information about new Delete function.

Added references to Peek being able to preview videos and folder summaries, in addition to being able to hover over previews for tooltip information.

Added information about new Delete function.

Added references to Peek being able to preview videos and folder summaries.
Copy link
Contributor

@daverayment : Thanks for your contribution! The author(s) have been notified to review your proposed change.

Copy link
Contributor

Learn Build status updates of commit efd4c48:

✅ Validation status: passed

File Status Preview URL Details
hub/powertoys/peek.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

@alvinashcraft alvinashcraft left a comment

Choose a reason for hiding this comment

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

Just a minor suggestion to turn the note into a Tip box. Which PT release will contain these updates?

hub/powertoys/peek.md Outdated Show resolved Hide resolved
Co-authored-by: Alvin Ashcraft <[email protected]>
Copy link
Contributor

Learn Build status updates of commit 4a1478a:

✅ Validation status: passed

File Status Preview URL Details
hub/powertoys/peek.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@daverayment
Copy link
Author

@alvinashcraft Thank you very much for the suggestion, which I've accepted and committed.

I'm afraid I've only just submitted the PR for the Delete functionality itself, so it hasn't been through code review or planned for inclusion in a release yet. This is the first time I've updated the docs along with a feature, so I don't know how the code and docs repos are aligned - should I comment again here once I know which version will include the code?

@alvinashcraft
Copy link
Contributor

Hi @daverayment. No worries. I have a few other PowerToys PRs that I've been holding onto while waiting for a release to go out. If you happen to hear when this gets assigned to a release, a comment here would be appreciated. Otherwise, I'll keep an eye on the status of your PR as each release goes out.

@Jay-o-Way
Copy link
Contributor

@daverayment thank for this pr. Please do link the related (PowerToys) PRs too, if you will? :)

@daverayment
Copy link
Author

@Jay-o-Way There is a mention to the dev PR earlier in the thread on October 13th. Sorry, I should have also included a link in the initial comment.

It's microsoft/PowerToys#35418

Copy link
Contributor

Learn Build status updates of commit cdc95f5:

✅ Validation status: passed

File Status Preview URL Details
hub/powertoys/peek.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

Learn Build status updates of commit c9d678c:

✅ Validation status: passed

File Status Preview URL Details
hub/images/pt-peek-delete-confirmation.png ✅Succeeded
hub/powertoys/peek.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

PRMerger Results

Issue Description
Added File(s) This PR contains added files. New files require human review.
File Change Percent This PR contains file(s) with more than 30% file change.
Image File This PR added or updated an image file(s).

@daverayment
Copy link
Author

@alvinashcraft @Jay-o-Way This updated PR includes additional documentation for the Peek Delete functionality. This was originally delayed from a prior PowerToys release to incorporate a few more requests from the code review, including a new confirmation dialog and a settings option.

I'm unsure whether the feature will land in 0.88, but I will ask on the main code PR and let you know.

Copy link
Contributor

@alvinashcraft alvinashcraft left a comment

Choose a reason for hiding this comment

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

A few suggestions inline. Mostly minor things. Thanks for the update!

hub/powertoys/peek.md Outdated Show resolved Hide resolved
hub/powertoys/peek.md Show resolved Hide resolved
hub/powertoys/peek.md Show resolved Hide resolved
hub/powertoys/peek.md Show resolved Hide resolved
hub/powertoys/peek.md Outdated Show resolved Hide resolved
Copy link
Contributor

Learn Build status updates of commit 11491a3:

⚠️ Validation status: warnings

File Status Preview URL Details
hub/docfx.json ⚠️Warning Details
hub/images/pt-peek-delete-confirmation.png ✅Succeeded
hub/powertoys/peek.md ✅Succeeded

hub/docfx.json

  • Line 47, Column 25: [Warning: ms-service-subservice-invalid - See documentation] Invalid value for 'ms.subservice': 'apps' is not valid with 'ms.service' value 'traceprocessor'.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

Copy link
Contributor

Learn Build status updates of commit c8203a7:

⚠️ Validation status: warnings

File Status Preview URL Details
hub/docfx.json ⚠️Warning Details
hub/images/pt-peek-delete-confirmation.png ✅Succeeded
hub/powertoys/peek.md ✅Succeeded

hub/docfx.json

  • Line 47, Column 25: [Warning: ms-service-subservice-invalid - See documentation] Invalid value for 'ms.subservice': 'apps' is not valid with 'ms.service' value 'traceprocessor'.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

Copy link
Contributor

@alvinashcraft alvinashcraft left a comment

Choose a reason for hiding this comment

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

Thanks, Dave! This looks good. I'll hang onto it until the update goes out in a release.

Copy link
Contributor

Learn Build status updates of commit 4e0ea59:

✅ Validation status: passed

File Status Preview URL Details
hub/images/pt-peek-delete-confirmation.png ✅Succeeded
hub/powertoys/peek.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

PRMerger Results

Issue Description
Added File(s) This PR contains added files. New files require human review.
File Change Percent This PR contains file(s) with more than 30% file change.
Image File This PR added or updated an image file(s).

@daverayment
Copy link
Author

Thank you, @alvinashcraft. I've made a small update to reflect a simplification to the new confirmation dialog. Hopefully that's the last of the changes 🤞

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.

3 participants