-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Shared UX] Implement side nav v2 feedback snippet #234187
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
[Shared UX] Implement side nav v2 feedback snippet #234187
Conversation
e1ba324 to
336cb57
Compare
|
@elasticmachine run docs-build |
730a300 to
8cf6631
Compare
|
/ci |
x-pack/solutions/search/test/serverless/functional/test_suites/navigation.ts
Show resolved
Hide resolved
|
Pinging @elastic/appex-sharedux (Team:SharedUX) |
TattdCodeMonkey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
search changes seem fine to me.
NOTICE.txt
Outdated
| THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | ||
|
|
||
| --- | ||
| This code is based on the `canvas-confetti` library under the ISC license. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elastic/kibana-security, the lib is single file 1000loc, would you prefer to copy into a package instead (while preserving the author notice)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer keeping this as a dependency for now. Thanks for bringing this up! It lead to a good discussion. Summary - the maintenance burden of a 3rd party dependency is preferable over that of additional codebase.
src/core/packages/chrome/navigation/src/components/feedback_snippet/confetti.tsx
Outdated
Show resolved
Hide resolved
src/core/packages/chrome/navigation/src/components/side_nav/panel.tsx
Outdated
Show resolved
Hide resolved
x-pack/solutions/search/test/serverless/functional/test_suites/navigation.ts
Show resolved
Hide resolved
8cf6631 to
769477c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking:
This component is highly reusable and should ideally live outside of specific UI elements like the navigation, e.g. in a shared location such as shared/packages/shared-ux, as @Dosant suggested for the feedback snippet. We can make the component consume canvasConfetti options through props.
I also see a potential to promote this to EUI but that's not for now.
942504a to
3e5cac4
Compare
src/platform/packages/shared/shared-ux/feedback_snippet/impl/src/feedback_snippet.tsx
Outdated
Show resolved
Hide resolved
1e3e656 to
33d7704
Compare
NOTICE.txt
Outdated
| THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | ||
|
|
||
| --- | ||
| This code is based on the `canvas-confetti` library under the ISC license. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer keeping this as a dependency for now. Thanks for bringing this up! It lead to a good discussion. Summary - the maintenance burden of a 3rd party dependency is preferable over that of additional codebase.
src/platform/packages/shared/shared-ux/feedback_snippet/impl/src/confetti.tsx
Outdated
Show resolved
Hide resolved
ed67a21 to
bce5b47
Compare
src/core/packages/chrome/navigation/src/components/side_nav/panel.tsx
Outdated
Show resolved
Hide resolved
…earch feedback test
…ation solutionId prop
b68df9b to
70665a8
Compare
| @@ -0,0 +1,23 @@ | |||
| # @kbn/shared-ux-feedback-snippet | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't really need /impl/ filepath for the pacakge. the impl and types separation is unnecessary with the current setup and was added when we had a more constrained package system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know, I can remove it in an upcoming PR 👍
fe6e4c4 to
8cb28e1
Compare
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
Total ESLint disabled count
History
cc @angeles-mb |
## Summary - Following security team advice and updating our backport strategy for recently added `canvas-confetti` dependency to `current-major`. - Once me move to `9.3` we should update this once again to `previous-minor`. Related PR: #234187
## Summary - Restructured feedback snippet dirs before making any more changes to this shared component so it matches our current structure - Based on a [suggestion](#234187 (comment)) on this PR #234187 ## Testing - Manually tested that the snippet is displayed correctly
Closes elastic/eui-private#385 ## Summary - Added a feedback snippet to the new side nav (v2) which can take the shape of: - A panel when seen for the first time - A button after any interaction with the panel occurs (dismiss, thumb up or thumb down) - Test it here in a solution specific space: https://angeles-mb-pr-234187-385-implement-side-nav-feedback-snippe.kbndev.co/ - Its goal is to gather user feedback in the form of: - Tracked click events (on this first iteration: EBT out of the box support to record clicks based on id on dismiss, thumb up, thumb down and open survey) - Qualtrics surveys - User interaction is stored in local storage - Feedback panel has 3 views: prompt, positive and negative - Added `confetti` functionality to the positive view from [canvas-confetti](https://www.npmjs.com/package/canvas-confetti) library - Included license notice and ran `node scripts/notice` to update NOTICE.txt - Purpose: adds confetti functionality to meet the design expectations for this component - Justification: ready to use confetti functionality to surprise and congratulate the user - Alternatives explored: this library was recommended by the team and lightweight - Existing dependencies: I didn't find anything similar - **Still TODO**: - Write UTs elastic#234445 ## Visuals Video contains following scenarios: 1. Dismiss panel on 'x' -> feedback button appears -> navigate to survey 2. Click on Thumb Up -> positive view -> feedback button appears -> navigate to survey 3. Click on Thumb Down -> negative view -> navigate away closing secondary menu -> open a secondary menu again -> feedback button appears 4. Click on Thumb Down -> negative view -> navigate to survey https://github.com/user-attachments/assets/8eb43915-71d9-479d-9360-2396a637bae2 --------- Co-authored-by: kibanamachine <[email protected]>
…#235175) ## Summary - Following security team advice and updating our backport strategy for recently added `canvas-confetti` dependency to `current-major`. - Once me move to `9.3` we should update this once again to `previous-minor`. Related PR: elastic#234187
## Summary - Restructured feedback snippet dirs before making any more changes to this shared component so it matches our current structure - Based on a [suggestion](elastic#234187 (comment)) on this PR elastic#234187 ## Testing - Manually tested that the snippet is displayed correctly
Closes elastic/eui-private#385 ## Summary - Added a feedback snippet to the new side nav (v2) which can take the shape of: - A panel when seen for the first time - A button after any interaction with the panel occurs (dismiss, thumb up or thumb down) - Test it here in a solution specific space: https://angeles-mb-pr-234187-385-implement-side-nav-feedback-snippe.kbndev.co/ - Its goal is to gather user feedback in the form of: - Tracked click events (on this first iteration: EBT out of the box support to record clicks based on id on dismiss, thumb up, thumb down and open survey) - Qualtrics surveys - User interaction is stored in local storage - Feedback panel has 3 views: prompt, positive and negative - Added `confetti` functionality to the positive view from [canvas-confetti](https://www.npmjs.com/package/canvas-confetti) library - Included license notice and ran `node scripts/notice` to update NOTICE.txt - Purpose: adds confetti functionality to meet the design expectations for this component - Justification: ready to use confetti functionality to surprise and congratulate the user - Alternatives explored: this library was recommended by the team and lightweight - Existing dependencies: I didn't find anything similar - **Still TODO**: - Write UTs #234445 ## Visuals Video contains following scenarios: 1. Dismiss panel on 'x' -> feedback button appears -> navigate to survey 2. Click on Thumb Up -> positive view -> feedback button appears -> navigate to survey 3. Click on Thumb Down -> negative view -> navigate away closing secondary menu -> open a secondary menu again -> feedback button appears 4. Click on Thumb Down -> negative view -> navigate to survey https://github.com/user-attachments/assets/8eb43915-71d9-479d-9360-2396a637bae2 --------- Co-authored-by: kibanamachine <[email protected]>
## Summary - Following security team advice and updating our backport strategy for recently added `canvas-confetti` dependency to `current-major`. - Once me move to `9.3` we should update this once again to `previous-minor`. Related PR: #234187
## Summary - Restructured feedback snippet dirs before making any more changes to this shared component so it matches our current structure - Based on a [suggestion](#234187 (comment)) on this PR #234187 ## Testing - Manually tested that the snippet is displayed correctly
Closes https://github.com/elastic/eui-private/issues/385
Summary
confettifunctionality to the positive view from canvas-confetti librarynode scripts/noticeto update NOTICE.txtVisuals
Video contains following scenarios:
Screen.Recording.2025-09-05.at.14.46.46.mov