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

[Incontext insights] clean up components and usability #182

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

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Apr 2, 2024

Description

Addresses some of the comments on the original PR for this component, with changes from user studies.

Added a UI Settings which can be modified in Advanced Settings and increase the timeout as one of the issues is that the original was too fast to see the initial state on load. Then I felt it was really long so made it configurable for users to decide whats best and for us to fine tune it with more insight.

Does not resolve every issue but I still plan on making it so a custom attribute that gets stylized instead of making plugins wrap components. But higher priorities have made me delay these changes. So opening to not let it get too stale.

Issues Partially Resolved

#143

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test.
  • New functionality has user manual doc added.
  • Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
@kavilla kavilla added the backport 2.x Trigger the backport flow to 2.x label Apr 2, 2024
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.92%. Comparing base (3991de2) to head (bdf0289).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #182      +/-   ##
==========================================
+ Coverage   90.02%   90.92%   +0.90%     
==========================================
  Files          54       61       +7     
  Lines        1433     1443      +10     
  Branches      347      347              
==========================================
+ Hits         1290     1312      +22     
+ Misses        141      129      -12     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SuZhou-Joe
Copy link
Member

Hi @kavilla , is it needed for 2.13?

@Hailong-am
Copy link
Collaborator

@kavilla would you mind rebase this PR with main to resolve the conflicts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Trigger the backport flow to 2.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants