feat(content-explorer): Add initial filter values to Metadata View #4225
Conversation
WalkthroughBumps several @box package versions in package.json and exposes a public-friendly Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant MVC as MetadataViewContainer
participant MV as MetadataView (ActionBar)
participant C as Consumer onFilterSubmit
Note over MVC: Accepts public actionBarProps
MVC->>MVC: transformInitialFilterValuesToInternal(publicValues)
MVC->>MV: render with transformedActionBarProps (initialFilterValues, onFilterSubmit)
U->>MV: interact with filters
MV-->>MVC: onFilterSubmit(internalFieldValues)
MVC->>MVC: transformInternalFieldsToPublic(internalFieldValues)
MVC-->>C: call consumer onFilterSubmit(publicValues)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/elements/content-explorer/MetadataViewContainer.tsx (2)
44-60: Consider simplifying the type casting logic.The current implementation uses multiple type assertions (
as unknown as) which reduces type safety. Consider a more type-safe approach.Apply this diff to improve type safety:
- const initialFilterValues = React.useMemo(() => { - const filterValues = actionBarProps?.initialFilterValues; - if (!filterValues) return undefined; - - const transformed: Record<string, { value: InternalMetadataFormFieldValue }> = {}; - Object.entries(filterValues).forEach(([key, filterValue]) => { - const { value } = filterValue as { value: unknown }; - if (Array.isArray(value)) { - // Convert customer-friendly array to internal enum shape - transformed[key] = { value: { enum: value } as unknown as InternalMetadataFormFieldValue }; - } else { - // Keep range/float as-is - transformed[key] = filterValue as unknown as { value: InternalMetadataFormFieldValue }; - } - }); - return transformed; - }, [actionBarProps?.initialFilterValues]); + const initialFilterValues = React.useMemo(() => { + const filterValues = actionBarProps?.initialFilterValues; + if (!filterValues) return undefined; + + const transformed: Record<string, { value: InternalMetadataFormFieldValue }> = {}; + Object.entries(filterValues).forEach(([key, filterValue]) => { + const { value } = filterValue; + if (Array.isArray(value)) { + // Convert customer-friendly array to internal enum shape + transformed[key] = { value: { enum: value } as InternalMetadataFormFieldValue }; + } else { + // Keep range/float as-is + transformed[key] = { value: value as InternalMetadataFormFieldValue }; + } + }); + return transformed; + }, [actionBarProps?.initialFilterValues]);
63-79: Improve type safety in the transformation logic.Similar to the initial filter values transformation, the type assertions here can be simplified for better type safety.
Apply this diff to improve type safety:
- const onFilterSubmit = React.useCallback( - (fields: Record<string, { value: InternalMetadataFormFieldValue }>) => { - if (!actionBarProps?.onFilterSubmit) return; - - const transformed: Record<string, { value: MetadataFormFieldValue }> = {}; - Object.entries(fields).forEach(([key, filterValue]) => { - const { value } = filterValue as { value: MetadataFormFieldValue }; - if (value && 'enum' in value && Array.isArray(value.enum)) { - transformed[key] = { value: value.enum }; - } else { - transformed[key] = { value }; - } - }); - actionBarProps.onFilterSubmit(transformed); - }, - [actionBarProps], - ); + const onFilterSubmit = React.useCallback( + (fields: Record<string, { value: InternalMetadataFormFieldValue }>) => { + if (!actionBarProps?.onFilterSubmit) return; + + const transformed: Record<string, { value: MetadataFormFieldValue }> = {}; + Object.entries(fields).forEach(([key, filterValue]) => { + const { value } = filterValue; + if (value && typeof value === 'object' && 'enum' in value && Array.isArray((value as any).enum)) { + transformed[key] = { value: (value as any).enum }; + } else { + transformed[key] = { value: value as MetadataFormFieldValue }; + } + }); + actionBarProps.onFilterSubmit(transformed); + }, + [actionBarProps], + );src/elements/content-explorer/types.ts (1)
7-12: Consider exporting FilterValues type.The FilterValues type is used in ActionBarProps but not exported. This could be useful for consumers who need to work with filter values directly.
Apply this diff to export the type:
-type FilterValues = Record< +export type FilterValues = Record< string, { value: MetadataFormFieldValue; } >;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
package.json(2 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(2 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(1 hunks)src/elements/content-explorer/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T18:04:17.698Z
Learnt from: tjuanitas
PR: box/box-ui-elements#4224
File: package.json:296-297
Timestamp: 2025-08-12T18:04:17.698Z
Learning: In the box-ui-elements project, the team is comfortable with raising peerDependency minimum versions when upgrading blueprint-web packages, even if it's a breaking change for consumers.
Applied to files:
package.json
🧬 Code Graph Analysis (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
src/elements/content-explorer/types.ts (2)
ActionBarProps(14-20)MetadataFormFieldValue(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint_test_build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Summary
🔇 Additional comments (5)
package.json (1)
139-139: Version bump looks good.The @box/metadata-view version update from ^0.29.4 to ^0.39.4 is consistent across all three locations (dependencies, devDependencies, and peerDependencies) and aligns with the new filter functionality.
src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx (2)
136-142: Well-structured test data for filter values.The initial filter values appropriately demonstrate the different value types supported: single string array for industry, multiple values for mimetype, and multiple values for role. This provides good test coverage for the filtering functionality.
144-168: Comprehensive visual test with proper assertions.The story effectively tests the initial filter values feature with appropriate waiting for content to render and assertions on the filter chip counts. The test validates that the UI correctly reflects the initial filter state.
src/elements/content-explorer/types.ts (2)
4-5: LGTM! Clean type definition.The MetadataFormFieldValue type clearly defines the supported filter value types with appropriate use of union types.
14-20: Well-designed ActionBarProps type.The type properly extends MetadataViewProps['actionBarProps'] while overriding specific properties with customer-friendly versions. This provides a clean API surface for consumers.
jfox-box
force-pushed
the
initial-filter-values
branch
from
ae1a0ea to
1b51b6d
Compare
jfox-box
changed the title
jfox-box
force-pushed
the
initial-filter-values
branch
from
4bfbe6e to
0adee80
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/elements/content-explorer/MetadataViewContainer.tsx (5)
3-4: Use type-only imports for types to avoid bundling and match local style.This file already uses type-only imports elsewhere. Importing types as values can cause unnecessary runtime code in certain TS configs.
-import { FloatType, MetadataFormFieldValue, RangeType } from '@box/metadata-filter'; +import type { FloatType, MetadataFormFieldValue, RangeType } from '@box/metadata-filter';
8-17: Future-proof the public filter value type by deriving it from the internal shape.Hard-coding the union risks drift if
MetadataFormFieldValueevolves. A conditional type keeps the public type in sync and automatically adapts to new non-enum shapes.-// Public-friendly metadata value shape (array value for enum type, range/float objects stay the same) -export type MetadataFormFieldValuePublic = string[] | RangeType | FloatType; - -export type FilterValuesPublic = Record< - string, - { - value: MetadataFormFieldValuePublic; - } ->; +// Public-friendly metadata value shape (array for enum, pass-through for other shapes) +type Publicify<T> = T extends { enum: infer E } ? E : T; +export type MetadataFormFieldValuePublic = Publicify<MetadataFormFieldValue>; + +export type FilterValuesPublic = Record<string, { value: MetadataFormFieldValuePublic }>;
18-24: Consider a less ambiguous name for the exported ActionBarProps alias.Since
actionBarPropsalso exists onMetadataViewProps, naming collisions in imports are easy. ConsiderExplorerActionBarProps(or similar) to clarify origin and avoid confusion in consumers importing both.
80-96: Tighten callback dependencies to reduce unnecessary re-creations.The callback only needs to re-memoize when
onFilterSubmitchanges, not the wholeactionBarPropsobject.- [actionBarProps], + [actionBarProps?.onFilterSubmit],
98-106: Only pass an onFilterSubmit handler when the consumer provided one.Avoids wiring up a no-op that still triggers the child’s submit pathway.
return { ...actionBarProps, initialFilterValues, - onFilterSubmit, + onFilterSubmit: actionBarProps?.onFilterSubmit ? onFilterSubmit : undefined, filterGroups, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/elements/content-explorer/MetadataViewContainer.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (2)
src/elements/content-explorer/MetadataViewContainer.tsx (2)
26-27: LGTM on public-facing props surface.Narrowing
actionBarPropsto the public shape while preserving the rest ofMetadataViewPropslooks correct.
107-107: LGTM on prop propagation.Passing the transformed action bar props into MetadataView is the right integration point.
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx
Outdated
Show resolved
Hide resolved
src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx
Outdated
Show resolved
Hide resolved
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx
Outdated
Show resolved
Hide resolved
jfox-box
force-pushed
the
initial-filter-values
branch
from
7e96419 to
0bb39ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx (3)
85-103: Strong, targeted test validating external multiSelect shape.This addition clearly asserts that onFilterSubmit receives a string[] for multiSelect (i.e., no internal { enum: [...] } wrapper). Nice coverage of the new public-facing contract.
If you want to round out coverage per the PR objectives, consider a companion test that passes initialFilterValues in the new public shape (e.g., role: ['Developer']) and asserts that:
- The chip renders the initial selections, and
- The first onFilterSubmit payload preserves string[] for that field.
I can draft that test on request.
106-111: Reduce flakiness by awaiting the menu with findByRole.After clicking the chip, the menu can render asynchronously. Using getByRole('menu') immediately can be flaky. Prefer findByRole to await the menu before querying within it.
Apply this diff:
- await userEvent().click(screen.getByRole('button', { name: /Contact Role/ })); - await userEvent().click(within(screen.getByRole('menu')).getByRole('menuitemcheckbox', { name: 'Developer' })); + await userEvent().click(screen.getByRole('button', { name: /Contact Role/ })); + await userEvent().click( + within(await screen.findByRole('menu')).getByRole('menuitemcheckbox', { name: 'Developer' }), + ); // Re-open the chip to select a second value (menu closes after submit) - await userEvent().click(screen.getByRole('button', { name: /Contact Role/ })); - await userEvent().click(within(screen.getByRole('menu')).getByRole('menuitemcheckbox', { name: 'Marketing' })); + await userEvent().click(screen.getByRole('button', { name: /Contact Role/ })); + await userEvent().click( + within(await screen.findByRole('menu')).getByRole('menuitemcheckbox', { name: 'Marketing' }), + );
112-117: Tighten assertions to focus on payload shape, not temp variables.You can assert the exact payloads directly and make the test more resilient to extra props in the object by using objectContaining with nth-called assertions.
Apply this diff:
- await waitFor(() => expect(onFilterSubmit).toHaveBeenCalledTimes(2)); - const firstCall = onFilterSubmit.mock.calls[0][0]; - const secondCall = onFilterSubmit.mock.calls[1][0]; - expect(firstCall['role-filter'].value).toEqual(['Developer']); - expect(secondCall['role-filter'].value).toEqual(['Developer', 'Marketing']); + await waitFor(() => expect(onFilterSubmit).toHaveBeenCalledTimes(2)); + expect(onFilterSubmit).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + 'role-filter': expect.objectContaining({ value: ['Developer'] }), + }), + ); + expect(onFilterSubmit).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + 'role-filter': expect.objectContaining({ value: ['Developer', 'Marketing'] }), + }), + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/elements/content-explorer/MetadataViewContainer.tsx(3 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(3 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/elements/content-explorer/MetadataViewContainer.tsx
- src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint_test_build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Summary
🔇 Additional comments (2)
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx (2)
5-6: Good import consolidation and alignment with test-utils.Importing userEvent, waitFor, and within from the local testing-library wrapper is consistent with team conventions and prior feedback. Looks good.
22-30: Minor template tweak looks fine.Changing the mock field id to 'field2' for the Industry enum field is fine and keeps the mock template coherent.
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx
Outdated
Show resolved
Hide resolved
jfox-box
force-pushed
the
initial-filter-values
branch
from
53ac55b to
f5cc509
Compare
Adds a change to Content Explorer Metadata View that changes
initalFilterValuesandonFilterSubmitto use a public-friendly version of the internal enum type field value format.field1: { value: { enum: ['value1', 'value2'] } }field1: { value: ['value1', 'value2'] } }Summary by CodeRabbit
New Features
Tests
Chores