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

feat(box-ai): Update Shared packages versions #3926

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

marcoartaviaq
Copy link
Contributor

@marcoartaviaq marcoartaviaq commented Feb 12, 2025

Update packages to their latest version.

Some fixes needs to be done at test level since Box AI went into a redesign that changed the style and copy of the text

@marcoartaviaq marcoartaviaq requested a review from a team as a code owner February 12, 2025 21:15
JChan106
JChan106 previously approved these changes Feb 12, 2025
submitQuestion={handleAsk}
suggestedQuestions={suggestedQuestions || localizedQuestions}
warningNotice={spreadsheetNotice}
warningNoticeAriaLabel={formatMessage(messages.welcomeMessageSpreadsheetNoticeAriaLabel)}
userInfo={userInfo}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New modal does not need userInfo nor onModalClose

Copy link
Contributor

Choose a reason for hiding this comment

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

how does modal know who the user is? like for the avatar circle in the conversation box?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Avatar circle wont appear anymore with lates design

onOpenChange={handleOnRequestClose}
open={isOpen}
questions={questions}
retryQuestion={handleRetry}
setAnswerFeedback={undefined}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This failed the lint step if we didn't declare a value. For the moment, we pass undefined in order to not show the feedback yet

@marcoartaviaq marcoartaviaq force-pushed the update-ai-package branch 4 times, most recently from a462d60 to 5d9b2bd Compare February 12, 2025 23:30
@@ -42,7 +42,6 @@ export interface ContentAnswersModalProps extends ExternalProps {

const ContentAnswersModal = ({
api,
currentUser,
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the currentUser from the prop type and the withCurrentUser at the bottom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

submitQuestion={handleAsk}
suggestedQuestions={suggestedQuestions || localizedQuestions}
warningNotice={spreadsheetNotice}
warningNoticeAriaLabel={formatMessage(messages.welcomeMessageSpreadsheetNoticeAriaLabel)}
userInfo={userInfo}
Copy link
Contributor

Choose a reason for hiding this comment

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

how does modal know who the user is? like for the avatar circle in the conversation box?

@@ -192,7 +192,6 @@ function BoxAISidebarContent(props: ApiWrapperProps) {
items={items}
itemSize={itemSize}
onClearAction={onClearAction}
onModalClose={handleModalClose}
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this modal need the same prop changes as the ContentAnswers file? such as setAnswerFeedback or userInfo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

box-ai-content-answers shared feature removed and added those props. This will apply for all uses in BUIE since the UI was updated

Copy link
Contributor

Choose a reason for hiding this comment

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

whoops, word spaghetti. i meant to say: why does it not need the same prop changes as the other file? this file still passes userInfo and does not pass setAnswerFeedback

tjuanitas
tjuanitas previously approved these changes Feb 13, 2025
Copy link
Contributor

@tjuanitas tjuanitas left a comment

Choose a reason for hiding this comment

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

lgtm. looks like the BoxAISidebarContent component can have some clean up with removing userInfo from being passed to the modal and removing from BoxAISidebarContext

@@ -8,7 +8,6 @@ import ContentAnswersModal from '../ContentAnswersModal';
import {
mockApi,
mockApiReturnError,
mockCurrentUser,
Copy link
Contributor

Choose a reason for hiding this comment

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

similar here. this mock can be removed from the mocks file

@tjuanitas tjuanitas merged commit dc5d352 into box:master Feb 13, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants