AP-25527: Use ai-settings for persisting K-AI disclaimer dismissal#112
Conversation
.knime/profile/localStorage) for persisting K-AI disclaimer dismissal158da5e to
9217096
Compare
9217096 to
c2ef54c
Compare
|
A small quality-of-life bit of work here, danke @hriverahdez 🙏 |
c2ef54c to
ac219b6
Compare
hriverahdez
left a comment
There was a problem hiding this comment.
@iusethemouse just marking this as request for changes in the meantime, to avoid an accidental merge.
ping me if/when you want me to re-review it
ac219b6 to
97f4bf2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ready for another look @hriverahdez |
a141e48 to
98ceb3f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const closeDisclaimer = async (persistently: boolean = true) => { | ||
| hasBeenDismissed.value = true; | ||
|
|
||
| if (localStorageKey.value && persistently) { | ||
| localStorage.setItem(localStorageKey.value, "true"); | ||
| if (persistently && disclaimerText.value) { | ||
| await aiSettingsStore.dismissDisclaimer(disclaimerText.value); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The closeDisclaimer function is async but may be called without awaiting by the consumer. While this won't cause an error, it means the UI could proceed before the dismissal is persisted to settings. If the user closes the app or tab immediately after calling this function, the dismissal might not be saved. Consider documenting this behavior or having callers await the promise to ensure persistence completes before allowing the user to proceed.
There was a problem hiding this comment.
jesus christ, this guy always has something to say...
|
Sorry for the continuous pings, ready again @hriverahdez Thanks for all the tips, things are in a much better place thanks to them |
…nd nothing else
af2143e to
6163e97
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|



Currently, when you click "Accept and continue" under K-AI's disclaimer in K-AI's sidepanel or in Quick Build mode with "Do not ask again" checked, the choice was persisted to
localStorage. This has always been problematic and brittle, often breaking, and currently not working in AP-in-browser.We recently introduced "AI settings" to knime-ui, which are persisted the same way as "UI settings" (i.e.
knime-ui-settings.yaml) - either in.knime/profileon Desktop AP, or inlocalStoragein browser. It makes sense to utilise this new persistence mechanism for K-AI's disclaimer as well.Core requirements for the disclaimer:
With these changes, I am expecting persistent K-AI disclaimer dismissal to finally work properly both in Dekstop AP and in browser.
Example of what
.knime/profile/ai-settings.yamllooks like after dismissing the disclaimer with "Do not ask again":, where
"3949c956"is the hash of username and Hub ID.Quick visual as a refresher of what "K-AI disclaimer" is:
