feat(metadata-sidebar): Add handler for Autofill button by jankowiakdawid · Pull Request #3700 · box/box-ui-elements

jankowiakdawid · 2024-10-07T16:59:08Z

No description provided.

And bump metadata-editor package Wrap MetadataInstanceList and MetadataInstanceForm in Autofill provider Co-authored-by: Wiola (wpiesiak)

JChan106 · 2024-10-10T18:05:50Z

+        async (templateKey: string, fields: MetadataTemplateField[]) => {
+            const aiAPI = api.getIntelligenceAPI();
+
+            await wait(1000);


Why do we have a setTimeout here? We should have a constant for the delay time as well.

sorry, I was using it to test loading state, and forgot to delete

greg-in-a-box · 2024-10-10T18:02:14Z

+        async (templateKey: string, fields: MetadataTemplateField[]) => {
+            const aiAPI = api.getIntelligenceAPI();
+
+            await wait(1000);


what is the reason for this ?

sorry, I was using it to test loading state, and forgot to delete

greg-in-a-box · 2024-10-10T18:07:07Z

    <AutofillContextProvider
-        isAiSuggestionsFeatureEnabled={isAiSuggestionsFeatureEnabled}
        fetchSuggestions={fetchSuggestions}
+        isAiSuggestionsFeatureEnabled={isAiSuggestionsFeatureEnabled}
    >


after looking at this again and noticed we are passing props specifically for it, think we need to consider moving this out of the testing-library into the metadata-editor tests directly, all the tests in the repo will use this wrapper eventually, i cant imagine that provider being used that much to warrant being in the wrapper. the other providers where are more generic

Agree. AutofillContextProvider is needed only for MetadataInstanceEditor tests.
I did tried moving it out but it resulted in failing tests

TypeError: (0 , _reactIntl.useIntl) is not a function

Not quite sure what's happening. But I needed to move the whole wrapper component to MetadataInstanceEditor test.

tjuanitas · 2024-10-10T18:05:42Z

            suggestionsResponse = await this.xhr.post({
                url,
                data: request,
+                id: `file_${request.items[0].id}`,


i see we do this in the ask method as well but how important is this id field? if items has multiple files would this cause conflicts or unexpected behavior since we always look at the first index?

id is used internally by post to get auth token, it can by an array, but for this endpoint we only use one file. That is whybI take first element.

tjuanitas · 2024-10-10T18:14:11Z

 }

-const MetadataInstanceEditor: React.FC<MetadataInstanceEditorProps> = ({
+export const MetadataInstanceEditor: React.FC<MetadataInstanceEditorProps> = ({


nit: if we're exporting for unit tests, we usually add the export at bottom with something like:

export { MetadataInstanceEditor as MetadataInstanceEditorComponent };

example:

box-ui-elements/src/elements/content-sidebar/Sidebar.js

Lines 342 to 343 in b6b61bf

export { Sidebar as SidebarComponent };

export default flow([withCurrentUser, withFeatureConsumer, withRouter])(Sidebar);

We actually don't need the withApiWrapper so I will restore the old way we were exporting here.

tjuanitas · 2024-10-10T18:17:50Z

+        async (templateKey: string, fields: MetadataTemplateField[]) => {
+            const aiAPI = api.getIntelligenceAPI();
+
+            await wait(1000);


what's the reasoning for this? are we adding an artificial loading time?

sorry, I was using it to test loading state, and forgot to delete

tjuanitas

comment but approved. please address in follow up if not fixed in this PR

tjuanitas · 2024-10-10T21:09:29Z


-type RenderConnectedOptions = Omit<RenderOptions, 'wrapper'> & {
+type RenderConnectedOptions = RenderOptions & {
+    // Omit<RenderOptions, 'wrapper'> & {


uncomment or remove?

jankowiakdawid requested review from a team as code owners October 7, 2024 16:59

jankowiakdawid commented Oct 7, 2024

View reviewed changes

Comment thread src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Outdated

wpiesiak reviewed Oct 8, 2024

View reviewed changes

Comment thread src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Outdated

wpiesiak reviewed Oct 8, 2024

View reviewed changes

Comment thread src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Outdated

wpiesiak reviewed Oct 8, 2024

View reviewed changes

Comment thread src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts

wpiesiak reviewed Oct 8, 2024

View reviewed changes

Comment thread src/elements/content-sidebar/MetadataInstanceEditor.tsx Outdated

jankowiakdawid force-pushed the handle-autofill branch 2 times, most recently from 0bed1bb to 21d879a Compare October 9, 2024 14:50

JChan106 reviewed Oct 9, 2024

View reviewed changes

Comment thread src/elements/content-sidebar/hooks/useSidebarMetadataFetcher.ts Outdated

jankowiakdawid added 3 commits October 10, 2024 19:41

feat(metadata-sidebar): Add handler for Autofill button

d88af63

feat(metadata-sidebar): Handle Error by rethrowing them to host app

1a39b96

feat(metadata-sidebar): Add error handling

6243548

And bump metadata-editor package Wrap MetadataInstanceList and MetadataInstanceForm in Autofill provider Co-authored-by: Wiola (wpiesiak)

jankowiakdawid force-pushed the handle-autofill branch from 3f6575d to 6243548 Compare October 10, 2024 17:51

JChan106 reviewed Oct 10, 2024

View reviewed changes

greg-in-a-box requested changes Oct 10, 2024

View reviewed changes

tjuanitas reviewed Oct 10, 2024

View reviewed changes

jankowiakdawid added 2 commits October 10, 2024 22:09

feat(metadata-sidebar): Remove unnecessary wait function

2bb0fc1

feat(metadata-sidebar): Restore old export in MetadataInstanceEditor

c6aba6d

jankowiakdawid added the ready-to-merge label Oct 10, 2024

tjuanitas previously approved these changes Oct 10, 2024

View reviewed changes

feat(metadata-sidebar): Move AutofillContextProvider out of test-utils

3d8ba70

jankowiakdawid dismissed tjuanitas’s stale review via 3d8ba70 October 10, 2024 21:16

jankowiakdawid force-pushed the handle-autofill branch from b17dc53 to 3d8ba70 Compare October 10, 2024 21:16

greg-in-a-box approved these changes Oct 10, 2024

View reviewed changes

tjuanitas approved these changes Oct 10, 2024

View reviewed changes

JChan106 approved these changes Oct 10, 2024

View reviewed changes

tjuanitas approved these changes Oct 10, 2024

View reviewed changes

Merge branch 'master' into handle-autofill

f449840

mergify Bot merged commit 95735e0 into box:master Oct 10, 2024

	export { Sidebar as SidebarComponent };
	export default flow([withCurrentUser, withFeatureConsumer, withRouter])(Sidebar);

Conversation

jankowiakdawid commented Oct 7, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tjuanitas Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tjuanitas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tjuanitas Oct 10, 2024 •

edited

Loading