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

added notification off docs #196

Merged
merged 6 commits into from
Mar 4, 2024
Merged

Conversation

captain-Akshay
Copy link
Contributor

Notes for Reviewers

This PR fixes #

Signed commits

  • Yes, I signed my commits.

Signed-off-by: captain-Akshay <[email protected]>
Copy link

netlify bot commented Jan 29, 2024

Deploy Preview for bejewelled-pegasus-b0ce81 ready!

Name Link
🔨 Latest commit 07c9d32
🔍 Latest deploy log https://app.netlify.com/sites/bejewelled-pegasus-b0ce81/deploys/65e1fcd1feca45000886fcd5
😎 Deploy Preview https://deploy-preview-196--bejewelled-pegasus-b0ce81.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@leecalcote
Copy link
Member

@nwanduka., how did @captain-Akshay do?

Copy link
Member

@leecalcote leecalcote left a comment

Choose a reason for hiding this comment

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

@Yashsharma1911 , it's unclear according to this documentation whether this silence notifications button is a per comment silencer for a silencer for all comments... in a design or for my entire user account? Does this setting only affect me or does it affect other users of this design? Do I need to own the design in order to use this?

What happens if I muted, people make comments, I unmute will I receive historical notifications for comments that were placed while I had silence on?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this. Two issues:

  1. poor quality screenshot.
  2. let's have pleasant, positive sample interactions.

Copy link
Member

Choose a reason for hiding this comment

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

+1

@@ -68,6 +68,12 @@ After a comment has been resolved, there might be situations where you need to r

<img src="./comments-unresolved.gif" alt="Unresolving comments in designer" width="600">

### Mute Comment Notifications

Customize your notification preferences to mute emails for comments on your design. When this feature is enabled, you won't receive email notifications for new comments on your design. This can be useful if you want to temporarily pause notifications or reduce email clutter. Follow the steps below to mute comment notifications
Copy link
Member

Choose a reason for hiding this comment

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

You don't "mute emails", you "mute notifications" or "mute email notifications".

@@ -68,6 +68,12 @@ After a comment has been resolved, there might be situations where you need to r

<img src="./comments-unresolved.gif" alt="Unresolving comments in designer" width="600">

### Mute Comment Notifications

Customize your notification preferences to mute emails for comments on your design. When this feature is enabled, you won't receive email notifications for new comments on your design. This can be useful if you want to temporarily pause notifications or reduce email clutter. Follow the steps below to mute comment notifications
Copy link
Member

Choose a reason for hiding this comment

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

"with this feature enabled"... no. Please change to "With notifications for new comments silenced, ..."

Copy link
Member

Choose a reason for hiding this comment

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

"you won't receive email notifications for new comments on your design." - what about existing comments and new threaded added?

@captain-Akshay, buckle down a description of the logic here - describe the behavior in various scenarios / under various conditions.

@leecalcote
Copy link
Member

@captain-Akshay you can do better. And I expect you to do better.

@leecalcote
Copy link
Member

@nwanduka, it seems that you might have missed this review. The comments that I've made could / should be made by you as either the first reviewer or the writer of this PR.

@@ -68,6 +68,12 @@ After a comment has been resolved, there might be situations where you need to r

<img src="./comments-unresolved.gif" alt="Unresolving comments in designer" width="600">

### Mute Comment Notifications
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this, Akshay. Looks great.

Little suggestion: instead of having a separate section for this feature, why don't we add it to the "enable email notifications" section since they're related? Having a subheading about muting comment notifications under a heading on Tips for using comments as a design review tool seems counter-intuitive. Plus the content looks like a rewrite of the "enable email notifications" section so it's kind of repetitive.

The image looks great as well. Well done.

@nwanduka
Copy link
Contributor

nwanduka commented Feb 2, 2024

@nwanduka, it seems that you might have missed this review. The comments that I've made could / should be made by you as either the first reviewer or the writer of this PR.

I started a review and didn't realize I hadn't submitted it. I was honestly confused when I kept getting the request to review the PR. All the while I was thinking to myself, "is no one else seeing the comment I left?" Now I know why 🤦🏽‍♀️. Anyway, lesson learned.
image

@Yashsharma1911
Copy link
Member

@Yashsharma1911 , it's unclear according to this documentation whether this silence notifications button is a per comment silencer for a silencer for all comments... in a design or for my entire user account? Does this setting only affect me or does it affect other users of this design? Do I need to own the design in order to use this?

What happens if I muted, people make comments, I unmute will I receive historical notifications for comments that were placed while I had silence on?

These are good questions and yes, this doc doesn't answer all of these questions, @captain-Akshay I'll help here with updating this doc

@iArchitSharma iArchitSharma added the pr/draft WIP/Draft pull request label Feb 9, 2024
@iArchitSharma
Copy link
Contributor

@Yashsharma1911 @captain-Akshay @nwanduka added draft label please remove this when your finished with your changes

@captain-Akshay
Copy link
Contributor Author

image

@captain-Akshay
Copy link
Contributor Author

// @Yashsharma1911 let me know if i missed anything

Signed-off-by: captain-Akshay <[email protected]>
Copy link
Contributor

@nwanduka nwanduka left a comment

Choose a reason for hiding this comment

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

Quite an improvement from the first draft. The content looks good to me 👍🏽.

How about the image? Did you change the comment in the screenshot to a more positive one? If that's done, then I think we're set to get this merged.

@leecalcote
Copy link
Member

@VictoryWekwa @iArchitSharma good to go?

Signed-off-by: captain-Akshay <[email protected]>
Copy link
Contributor

@iArchitSharma iArchitSharma left a comment

Choose a reason for hiding this comment

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

It's good for merging once the image is repositioned.

{{< /alert >}}


<img src="./comment-notificationBell.png" alt="Turn Off notification from comments in designer" style="width:auto">
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please place the image above What Muting Affects:

Signed-off-by: Archit Sharma <[email protected]>
Copy link
Contributor

@iArchitSharma iArchitSharma left a comment

Choose a reason for hiding this comment

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

did it myself, good to merge!
Thank you @captain-Akshay

@iArchitSharma iArchitSharma merged commit 489c2c6 into layer5io:master Mar 4, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/kanvas-docs pr/draft WIP/Draft pull request
Development

Successfully merging this pull request may close these issues.

5 participants