Skip to content
This repository was archived by the owner on Apr 23, 2025. It is now read-only.

Conversation

sbouchet
Copy link
Collaborator

What is the purpose of this change? What does it change?

Added a toolbar icon/action to share feedback using telemetry backend

Was the change discussed in an issue?

fixes #554

How to test changes?

@sbouchet sbouchet requested a review from adietish July 11, 2023 12:50
@openshift-ci openshift-ci bot requested a review from jeffmaury July 11, 2023 12:50
@sbouchet
Copy link
Collaborator Author

/override ci/prow/e2e-4.11

@openshift-ci
Copy link

openshift-ci bot commented Jul 11, 2023

@sbouchet: Overrode contexts on behalf of sbouchet: ci/prow/e2e-4.11

In response to this:

/override ci/prow/e2e-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@adietish
Copy link
Contributor

adietish commented Jul 12, 2023

I see the top section of the feedback dialog using a different font than the rest of the dialog:
image
I think that the dialog title bar would better show priorities if the icons and title would be reshuffled. Here's a suggestion:
image

@sbouchet
Copy link
Collaborator Author

I see the top section of the feedback dialog using a different font than the rest of the dialog: image I think that the dialog title bar would better show priorities if the icons and title would be reshuffled. Here's a suggestion: image

it's a webpage, taken from the vscode extension. @mohitsuman can you comment ?

@sbouchet sbouchet requested review from mohitsuman and removed request for jeffmaury July 12, 2023 10:23
@adietish
Copy link
Contributor

I would also layout the labels/text area differently, following the jetbrains design guide
image

@adietish
Copy link
Contributor

it's a webpage, taken from the vscode extension. @mohitsuman can you comment ?

If it's a web-page, why not open it in the browser like jetbrains do (choosing the menu item opens their web form)?
image

image

@adietish
Copy link
Contributor

adietish commented Jul 12, 2023

I have another suggestion: On top of the whole form I'd add a way to rate the product in a single click, like one can find in the jetbrains form, on amazon product reviews etc.
image
My justification is that I usually dont like "wasting" my time providing feedback (I especially dislike writing in free text areas. I only do it if I have a bug, a problem that I want to be fixed). But when I do it, I do it if I can to do it very quickly. With a star rating, a single click is enough. I'd think most developers are in this state of mind. We could thus increase feedback rates by providing the simplest possible feedback: a star-rating. And those that feel like providing more, they still can with the additional entries in the form.

@sbouchet
Copy link
Collaborator Author

sbouchet commented Jul 12, 2023

by "webpage", i was referring to the webview implemented in vscode ( using a webpage input ) .
Your review made me think about removing that in favor of pure jface components.
Capture d’écran de 2023-07-12 16-59-07

@sbouchet sbouchet marked this pull request as draft July 12, 2023 15:01
@sbouchet
Copy link
Collaborator Author

I have another suggestion: On top of the whole form I'd add a way to rate the product in a single click, like one can find in the jetbrains form, on amazon product reviews etc. image My justification is that I usually dont like "wasting" my time providing feedback (I especially dislike writing in free text areas. I only do it if I have a bug, a problem that I want to be fixed). But when I do it, I do it if I can to do it very quickly. With a star rating, a single click is enough. I'd think most developers are in this state of mind. We could thus increase feedback rates by providing the simplest possible feedback: a star-rating. And those that feel like providing more, they still can with the additional entries in the form.

overall, the first 3 items are mandatory, and for the sliderss, the comment area shows up only if score is below 3, but comment is not mandatory.
i'll try to make the first one mandatory only.

@sbouchet
Copy link
Collaborator Author

Capture d’écran de 2023-07-13 16-58-04

dialog is now looking like this.

@adietish
Copy link
Contributor

@sbouchet: I simplified code that shows/hides components, made links for github and marketplace use non-serifed font and changed the positioning a bit. Hope you like it, otherwise just pick what the changes that you agree with and throw out the rest.
image

@sbouchet sbouchet marked this pull request as ready for review July 17, 2023 09:50
@openshift-ci openshift-ci bot requested a review from jeffmaury July 17, 2023 09:50
@sbouchet
Copy link
Collaborator Author

thank for the contributions ! i've added a margin to the left to better visual feedback when scrollbar is showed.

@sbouchet
Copy link
Collaborator Author

/override ci/prow/e2e-4.11

@openshift-ci
Copy link

openshift-ci bot commented Jul 17, 2023

@sbouchet: Overrode contexts on behalf of sbouchet: ci/prow/e2e-4.11

In response to this:

/override ci/prow/e2e-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sbouchet
Copy link
Collaborator Author

/override ci/prow/e2e-4.11

@openshift-ci
Copy link

openshift-ci bot commented Jul 18, 2023

@sbouchet: Overrode contexts on behalf of sbouchet: ci/prow/e2e-4.11

In response to this:

/override ci/prow/e2e-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Contributor

@adietish adietish left a comment

Choose a reason for hiding this comment

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

I like your screenshot a lot. Quite an improvement, visually and usability-wise.
On MacOS though, icons are cropped. Futhermore I am wondering if "Have you used any similar extension..." wasn't better off with checkboxes, too?
image

adietish and others added 2 commits August 22, 2023 16:04
@sbouchet
Copy link
Collaborator Author

sbouchet commented Aug 22, 2023

Capture d’écran du 2023-08-22 18-17-26

new version with :

  • aligned radio buttons for numbers
  • radio for Yes/No similar extension question

image ratio is OK on Fedora with fix in ce6c029

@sbouchet
Copy link
Collaborator Author

/override ci/prow/e2e-4.11

@openshift-ci
Copy link

openshift-ci bot commented Aug 22, 2023

@sbouchet: Overrode contexts on behalf of sbouchet: ci/prow/e2e-4.11

In response to this:

/override ci/prow/e2e-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Contributor

@adietish adietish left a comment

Choose a reason for hiding this comment

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

In terms of widgets and icons/images it's now great imho. But I noticed a weird behavior where I'm wondering if it's intentional:

Steps:

  1. ASSERT: "OK" button is disabled
  2. EXEC: Provide some text in "4. What, if anything..."
  3. ASSERT: "OK" button is disabled
  4. EXEC: provide some text that's not an email (ex. "dddd") in "Share your contact..."
  5. ASSERT: "OK" button is disabled, text field gets red with a balloon "The email format should be..."
  6. EXEC: delete the text in "Share your contact..."

Result:
"OK" button is enabled.
image

Expected Result:
If providing text in "4. What, if anything..." is enough to have a valid form, shouldn't the "OK" button get enabled as soon as I provide text to it, not just after filling in invalid email and removing it again?

Signed-off-by: Stephane Bouchet <[email protected]>
@sbouchet
Copy link
Collaborator Author

Fixed it. good catch !

@sbouchet
Copy link
Collaborator Author

/override ci/prow/e2e-4.11

@openshift-ci
Copy link

openshift-ci bot commented Aug 23, 2023

@sbouchet: Overrode contexts on behalf of sbouchet: ci/prow/e2e-4.11

In response to this:

/override ci/prow/e2e-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sbouchet
Copy link
Collaborator Author

@mohitsuman see above comments for rendering on both Mac (from @adietish) and Linux/Fedora (@sbouchet)

@adietish
Copy link
Contributor

@sbouchet I can still cause the "OK" button to get enabled when I add an invalid email and remove it again. Am I missing something?

Copy link
Contributor

@mohitsuman mohitsuman left a comment

Choose a reason for hiding this comment

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

The UI looks good to merge.

@openshift-ci openshift-ci bot added the lgtm label Aug 24, 2023
@adietish
Copy link
Contributor

The UI looks good to merge.

Yes, I agree, but you're still able to send empty forms.

@openshift-ci openshift-ci bot removed the lgtm label Aug 24, 2023
@openshift-ci
Copy link

openshift-ci bot commented Aug 24, 2023

New changes are detected. LGTM label has been removed.

@adietish
Copy link
Contributor

@sbouchet added a check for content. OK is not enabled if there's no content. Please review.

@adietish
Copy link
Contributor

@sbouchet added a check which makes sure that there's content in any of the widgets. Please review.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@sbouchet
Copy link
Collaborator Author

@sbouchet added a check which makes sure that there's content in any of the widgets. Please review.

nice ! works like a charm :)

@sbouchet
Copy link
Collaborator Author

/override ci/prow/e2e-4.11

@openshift-ci
Copy link

openshift-ci bot commented Aug 25, 2023

@sbouchet: Overrode contexts on behalf of sbouchet: ci/prow/e2e-4.11

In response to this:

/override ci/prow/e2e-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sbouchet sbouchet merged commit 073c528 into redhat-developer:main Aug 25, 2023
@sbouchet sbouchet deleted the issue-554 branch August 25, 2023 09:13
sbouchet added a commit to sbouchet/intellij-openshift-connector that referenced this pull request Aug 29, 2023
* feat: add feedback dialog

Signed-off-by: Stephane Bouchet <[email protected]>

* feat: add feedback dialog

fix sonar issues

Signed-off-by: Stephane Bouchet <[email protected]>

* feat: add feedback dialog

fix after review

Signed-off-by: Stephane Bouchet <[email protected]>

* feat: add feedback dialog

fix after sonar review

Signed-off-by: Stephane Bouchet <[email protected]>

* changed font and pos of links, simplified code

Signed-off-by: Andre Dietisheim <[email protected]>

* feat: add feedback dialog

added left margin

Signed-off-by: Stephane Bouchet <[email protected]>

* feat: add feedback dialog

Signed-off-by: Stephane Bouchet <[email protected]>

* feat: add feedback dialog

adjusted image size according to scale info

Signed-off-by: Stephane Bouchet <[email protected]>

* feat: add feedback dialog

Signed-off-by: Stephane Bouchet <[email protected]>

* fixed image scaling for MacOS (redhat-developer#554)

Signed-off-by: Andre Dietisheim <[email protected]>

* feat: add feedback dialog

Signed-off-by: Stephane Bouchet <[email protected]>

* feat: add feedback dialog

Signed-off-by: Stephane Bouchet <[email protected]>

* make sure form cannot be sent without content (redhat-developer#554)

Signed-off-by: Andre Dietisheim <[email protected]>

---------

Signed-off-by: Stephane Bouchet <[email protected]>
Signed-off-by: Andre Dietisheim <[email protected]>
Co-authored-by: Andre Dietisheim <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add feedback section for the extension
3 participants