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

[Epic] User Feedback Annotations #63749

Open
23 of 32 tasks
Tracked by #3602 ...
ryan953 opened this issue Jan 24, 2024 · 2 comments
Open
23 of 32 tasks
Tracked by #3602 ...

[Epic] User Feedback Annotations #63749

ryan953 opened this issue Jan 24, 2024 · 2 comments

Comments

@ryan953
Copy link
Member

ryan953 commented Jan 24, 2024

The goal of the project is to close a large feature request, sentry users want to allow their customers to create an mark-up screen shots to better illustrate their feedback problems.

Basically, we want to solve for the cases in getsentry/sentry-javascript#9460

KTTR Tracking

Design Files

Prep

Core - SDK (MVP)

  1. ryan953

Core - App (MVP)

  1. michellewzhang ryan953

Screenshots (GA)

Bugs

  1. ryan953
  2. ryan953
  3. ryan953
  4. c298lee

The goal of the project is to close a large feature request, sentry users want to allow their customers to create an mark-up screen shots to better illustrate their feedback problems.

Basically, we want to solve for the cases in getsentry/sentry-javascript#9460

KTTR Tracking

Design File

Annotations

  1. Feature: User Feedback
@ryan953 ryan953 changed the title [Epic] Feedback Screenshots & Annotations [Epic] User Feedback Screenshots & Annotations Jan 24, 2024
@c298lee c298lee self-assigned this Jan 25, 2024
@jas-kas jas-kas changed the title [Epic] User Feedback Screenshots & Annotations [Epic] User Feedback Screenshots Feb 26, 2024
@jas-kas jas-kas changed the title [Epic] User Feedback Screenshots [Epic] User Feedback Screenshots & Annotations Feb 26, 2024
c298lee added a commit that referenced this issue Mar 7, 2024
Adds ability to view screenshot attachments in feedbacks. The screenshot
preview and modal is the same as in issues, and the code is copied over
from EventTagsAndScreenshots. This will be modified when user feedback
is revamped.

Screenshot in feedback item:
<img width="955" alt="image"
src="https://github.com/getsentry/sentry/assets/55311782/1a704f26-6152-4471-be1e-766c7a0d9aa7">

Clicking on view screenshot:
<img width="714" alt="image"
src="https://github.com/getsentry/sentry/assets/55311782/6888821c-ae71-4ca8-a257-fb295e5464ce">

Relates to: #63749
c298lee added a commit to getsentry/sentry-javascript that referenced this issue Mar 21, 2024
Fixes issue where screenshotting in Safari results in black bars on the
right and bottom of the image.

Before:
<img width="1278" alt="image"
src="https://github.com/getsentry/sentry-javascript/assets/55311782/b2c174ad-8403-459a-b24b-7e055da3d657">

After:
<img width="1278" alt="image"
src="https://github.com/getsentry/sentry-javascript/assets/55311782/85fb850d-6c01-48cd-9fe0-21a860114b9d">

Relates to getsentry/sentry#63749
c298lee added a commit to getsentry/sentry-javascript that referenced this issue Mar 22, 2024
Fixes issue where screenshotting in Safari results in black bars on the
right and bottom of the image.

Before:
<img width="1278" alt="image"
src="https://github.com/getsentry/sentry-javascript/assets/55311782/b2c174ad-8403-459a-b24b-7e055da3d657">

After:
<img width="1278" alt="image"
src="https://github.com/getsentry/sentry-javascript/assets/55311782/85fb850d-6c01-48cd-9fe0-21a860114b9d">

Relates to getsentry/sentry#63749
billyvg added a commit to getsentry/sentry-javascript that referenced this issue Mar 25, 2024
…11152) (#11153)

Fix #11152

This introduces an option `isRequiredLabel`, and I planned to also add
`errorMessageText`.

However, this PR is branched off from a version that is a couple days
old. Right now, @c298lee is currently working on getsentry/sentry#63749,
which is causing major changes and the current version on master doesn't
work yet. Therefore, I didn't yet implement `errorMessageText`.

So consider this PR as a PoC, and either feel free to tag me when the
screenshot changes are done – then I'll redo the changes based on the
version that supports screenshots – or add the option on your own;
however you prefer 🙂

One open question:

Until now, there was only the error message:

> There was a problem submitting feedback, please wait and try again.

Now, depending on the status code, we have three error messages:

> - 'Unable to send Feedback. This is because of network issues, or
because you are using an ad-blocker.'
> - 'Unable to send Feedback. Invalid response from server.'
> - 'Unable to send Feedback'

Do you have a suggestion how we could make the message configurable,
without introducing too many and redundant settings? Maybe we should go
back to only one message? I'm not sure if an end-user cares about
whether it is a network issue or a server error.

---------

Co-authored-by: Billy Vong <billyvg@users.noreply.github.com>
c298lee added a commit to getsentry/sentry-javascript that referenced this issue Mar 26, 2024
… with designs (#11227)

Updated some screenshot styles to align with design:
- Changed cropping to more closely follow photoshop: darker background,
white cropping corners + rectangle and additional black border for
contrast if the screenshot is white
- Changed naming to follow BES naming
- Added padding so that the submit/cancel buttons are always visible
without scrolling
- Reduced blurriness of cropping rectangle
<img width="1270" alt="image"
src="https://github.com/getsentry/sentry-javascript/assets/55311782/df73a537-58f9-4f03-9edc-403039e61122">

Relates to getsentry/sentry#63749

---------
billyvg added a commit to getsentry/sentry-javascript that referenced this issue Mar 27, 2024
…le (#11287)

Backport of #11153

From the OP (h/t @soerface):

This introduces an option `isRequiredLabel`, and I planned to also add
`errorMessageText`.

However, this PR is branched off from a version that is a couple days
old. Right now, @c298lee is currently working on getsentry/sentry#63749,
which is causing major changes and the current version on master doesn't
work yet. Therefore, I didn't yet implement `errorMessageText`.

So consider this PR as a PoC, and either feel free to tag me when the
screenshot changes are done – then I'll redo the changes based on the
version that supports screenshots – or add the option on your own;
however you prefer 🙂

---------

Co-authored-by: Soeren Wegener <wegener92@gmail.com>
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this issue Apr 19, 2024
Fixes issue where screenshotting in Safari results in black bars on the
right and bottom of the image.

Before:
<img width="1278" alt="image"
src="https://github.com/getsentry/sentry-javascript/assets/55311782/b2c174ad-8403-459a-b24b-7e055da3d657">

After:
<img width="1278" alt="image"
src="https://github.com/getsentry/sentry-javascript/assets/55311782/85fb850d-6c01-48cd-9fe0-21a860114b9d">

Relates to getsentry/sentry#63749
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this issue Apr 19, 2024
…etsentry#11152) (getsentry#11153)

Fix getsentry#11152

This introduces an option `isRequiredLabel`, and I planned to also add
`errorMessageText`.

However, this PR is branched off from a version that is a couple days
old. Right now, @c298lee is currently working on getsentry/sentry#63749,
which is causing major changes and the current version on master doesn't
work yet. Therefore, I didn't yet implement `errorMessageText`.

So consider this PR as a PoC, and either feel free to tag me when the
screenshot changes are done – then I'll redo the changes based on the
version that supports screenshots – or add the option on your own;
however you prefer 🙂

One open question:

Until now, there was only the error message:

> There was a problem submitting feedback, please wait and try again.

Now, depending on the status code, we have three error messages:

> - 'Unable to send Feedback. This is because of network issues, or
because you are using an ad-blocker.'
> - 'Unable to send Feedback. Invalid response from server.'
> - 'Unable to send Feedback'

Do you have a suggestion how we could make the message configurable,
without introducing too many and redundant settings? Maybe we should go
back to only one message? I'm not sure if an end-user cares about
whether it is a network issue or a server error.

---------

Co-authored-by: Billy Vong <billyvg@users.noreply.github.com>
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this issue Apr 19, 2024
… with designs (getsentry#11227)

Updated some screenshot styles to align with design:
- Changed cropping to more closely follow photoshop: darker background,
white cropping corners + rectangle and additional black border for
contrast if the screenshot is white
- Changed naming to follow BES naming
- Added padding so that the submit/cancel buttons are always visible
without scrolling
- Reduced blurriness of cropping rectangle
<img width="1270" alt="image"
src="https://github.com/getsentry/sentry-javascript/assets/55311782/df73a537-58f9-4f03-9edc-403039e61122">

Relates to getsentry/sentry#63749

---------
@ryan953
Copy link
Member Author

ryan953 commented May 13, 2024

We've delievered the Screenshots portion of this, but the annotations stuff has been deferred until we get some feedback. Unassigning and setting this back to TODO. Also renamed to better track the remaining work.

@ryan953 ryan953 changed the title [Epic] User Feedback Screenshots & Annotations [Epic] User Feedback Annotations May 13, 2024
@ryan953
Copy link
Member Author

ryan953 commented May 13, 2024

POC: getsentry/sentry-javascript#11175

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

No branches or pull requests

2 participants