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

Sentiment feedback #1412

Merged
merged 2 commits into from
Nov 25, 2024
Merged

Sentiment feedback #1412

merged 2 commits into from
Nov 25, 2024

Conversation

TamiTakamiya
Copy link
Contributor

Jira Issue: https://issues.redhat.com/browse/AAP-33572

Description

Sentiment feedback (ThumbsUp/Down) UI implementation

Testing

Steps to test

  1. Pull down the PR
  2. Run unit tests

Scenarios tested

(Unit tests will be provided soon...)

Production deployment

  • This code change is ready for production on its own
  • This code change requires the following considerations before going to production:

Copy link
Contributor

@romartin romartin left a comment

Choose a reason for hiding this comment

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

As commented, please requesting also to update the call to the wisdom service's Feedback API accordingly.

Thanks @TamiTakamiya , great stuff ,we're close :D

@TamiTakamiya TamiTakamiya force-pushed the TamiTakamiya/AAP-33572/feedback-ui branch 4 times, most recently from 5cc0d45 to a70c403 Compare November 22, 2024 16:37
@TamiTakamiya
Copy link
Contributor Author

@romartin There were a few bugs in property names, etc. and I have fixed them. I ran this latest version with your service-side PR and verified that it could send a correct ChatFeedback request. If you need a chatbot client for testing your code, please update the client code with this branch. Thanks!

@romartin
Copy link
Contributor

Cool, thanks @TamiTakamiya , will do!

BTW let me know once this is ready for review as well! anyway if any early issues found, will let u know too.

@TamiTakamiya TamiTakamiya force-pushed the TamiTakamiya/AAP-33572/feedback-ui branch from e033ac7 to 69e4aea Compare November 22, 2024 18:42
@TamiTakamiya TamiTakamiya marked this pull request as ready for review November 22, 2024 19:07
@romartin
Copy link
Contributor

Hey @TamiTakamiya !

Tested against my PR #1413

The telemetry works well, so the request from the chatbot UI is successful 👍

On the other hand, once clicking on the thumbs up/down icons, although it sends the feedback, the icon is NOT changing the "clicked" style that let the user know that it has been clicked, and the feedback properly sent. Same way, after clicking, the icon clicked style should be preserved for that prompt/response.

Just once fixing this small UI thing, I think we are good to go. Thanks!

@TamiTakamiya
Copy link
Contributor Author

Hey @TamiTakamiya !

Tested against my PR #1413

The telemetry works well, so the request from the chatbot UI is successful 👍

On the other hand, once clicking on the thumbs up/down icons, although it sends the feedback, the icon is NOT changing the "clicked" style that let the user know that it has been clicked, and the feedback properly sent. Same way, after clicking, the icon clicked style should be preserved for that prompt/response.

Just once fixing this small UI thing, I think we are good to go. Thanks!

I spent an hour on this and tried to find a way to implement this, but could not find a handy way... It is not a small UI thing.... (at least for me :-)

These icons are implemented as buttons, not links, by PatternFly chatbot widget. So it does not remember whether it was visited or not. It is possible to specify additional CSS class name to each action (see attached image), but they need to be controlled by ourselves.

image

I have opened AAP-36568 for that work and want to implement it using that separated ticket. If it is ok, would you approve this PR? Thanks!

jameswnl
jameswnl previously approved these changes Nov 22, 2024
@TamiTakamiya
Copy link
Contributor Author

@jameswnl @romartin I have update the code and integrated the additional GitHub feedback logic when a user clicked the ThumbsDown button. The new code will change the background of clicked icon and once either ThumbsUp or ThumbDown is clicked, both of them for a specifc bot response are disabled.

Screenshare.-.2024-11-22.9_42_04.PM.mp4

Would you take a look at those changes again? Thanks!

Copy link
Contributor

@jameswnl jameswnl left a comment

Choose a reason for hiding this comment

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

Looks great!

@jameswnl jameswnl requested a review from romartin November 23, 2024 04:09
@TamiTakamiya
Copy link
Contributor Author

As commented, please requesting also to update the call to the wisdom service's Feedback API accordingly.

Thanks @TamiTakamiya , great stuff ,we're close :D
@romartin I believe those requested API changes are included. Pls review this again. Thanksl.

@TamiTakamiya TamiTakamiya merged commit 6b83250 into main Nov 25, 2024
10 checks passed
@TamiTakamiya TamiTakamiya deleted the TamiTakamiya/AAP-33572/feedback-ui branch November 25, 2024 13:07
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.

3 participants