fix(metadata-view): Enable sorting dropdown #4265
Conversation
WalkthroughI pity the fool who misses this: metadata view props flattened (top-level selectedKeys/onSelectionChange), getMetadataViewProps typing changed, MetadataViewContainer filters fields by visible columns and adds onSortChange/handleSortChange, and tests updated for column-driven filtering and sorting. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CE as ContentExplorer
participant MVC as MetadataViewContainer
Note over CE,MVC #DDEBF7: Selection flow (flattened API)
User->>MVC: Select/Deselect rows
MVC-->>CE: onSelectionChange(ids)
CE->>CE: Update selectedItemIds
alt Selection empty (not "all")
CE->>CE: Set isMetadataSidePanelOpen = false
end
CE-->>MVC: Provide selectedKeys
sequenceDiagram
actor User
participant AB as ActionBar
participant MVC as MetadataViewContainer
participant INT as InternalSort
participant EXT as ExternalConsumer
Note over AB,MVC #FFF2CC: Sorting flow via action bar
User->>AB: Change sort (column, direction)
AB->>MVC: handleSortChange({column, direction})
MVC->>INT: onSortChange(trim(column), mapDir(ASC/DESC))
alt External onSortChange provided
MVC->>EXT: onSortChange(originalColumn, originalDirection)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
✨ 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx (1)
271-279: Fix MSW sort condition and verify sorting, fool!
- In
src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsxMSW handler (lines 271–279), replace the hardcodedorderByFieldKey === 'industry'check with a full-key match (e.g.orderByFieldKey === \${metadataFieldNamePrefix}.industry`ororderByFieldKey.endsWith('.industry')) and normalizeorderByDirection` to uppercase.- In
metadataViewV2SortsFromHeader.play, afteruserEvent.click(industryHeader), add an assertion that the first row changes (for example,expect(await canvas.findByRole('row', { name: /Child 1/i })).toBeInTheDocument()).
🧹 Nitpick comments (2)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
227-227: Tiny type nit: tighten direction to a union.If you want extra type safety, use
'ASC' | 'DESC'instead ofstringin theonSortChangeprop type.- onSortChange?: (sortBy: Key, sortDirection: string) => void; + onSortChange?: (sortBy: Key, sortDirection: 'ASC' | 'DESC') => void;src/elements/content-explorer/ContentExplorer.tsx (1)
1615-1629: Flattened selection plumbing looks right; stabilize the handler, fool.Logic is correct (guards
'all', closes panel on empty). To avoid re-creating the callback every render and churning children, hoist it to an instance method.Apply within this hunk:
- const { onSelectionChange } = metadataViewProps ?? {}; const { selectedItemIds } = this.state; return { ...metadataViewProps, selectedKeys: selectedItemIds, - onSelectionChange: (ids: Selection) => { - onSelectionChange?.(ids); - const isSelectionEmpty = ids !== 'all' && ids.size === 0; - this.setState({ - selectedItemIds: ids, - ...(isSelectionEmpty && { isMetadataSidePanelOpen: false }), - }); - }, + onSelectionChange: this.handleMetadataSelectionChange, };Add this class field (outside the shown range):
private handleMetadataSelectionChange = (ids: Selection) => { this.props.metadataViewProps?.onSelectionChange?.(ids); const isSelectionEmpty = ids !== 'all' && ids.size === 0; this.setState({ selectedItemIds: ids, ...(isSelectionEmpty && { isMetadataSidePanelOpen: false }), }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP (Model Context Protocol) integration is disabled
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
package.json(2 hunks)src/elements/content-explorer/ContentExplorer.tsx(1 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(2 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(2 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(0 hunks)src/elements/content-explorer/stories/MetadataView.stories.tsx(1 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- src/elements/content-explorer/tests/MetadataViewContainer.test.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
📚 Learning: 2025-08-28T15:38:35.122Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
Applied to files:
src/elements/content-explorer/stories/MetadataView.stories.tsxsrc/elements/content-explorer/__tests__/ContentExplorer.test.tsxsrc/elements/content-explorer/MetadataViewContainer.tsx
📚 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
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
Applied to files:
src/elements/content-explorer/ContentExplorer.tsx
🔇 Additional comments (7)
package.json (1)
139-139: Version bump verified, fool!No
allowSortingprops remain; single@box/[email protected]resolved.src/elements/content-explorer/MetadataViewContainer.tsx (2)
221-240: Wrapper logic looks solid.Trims metadata prefix for internal API and forwards the full id to external
tableProps.onSortChange. Clean split. Respect.
245-246: Good rewire for table header sorting.Passing the wrapper into
tableProps.onSortChangeis the right play.src/elements/content-explorer/__tests__/ContentExplorer.test.tsx (2)
475-475: Top-level selection flag is correct.
isSelectionEnabledonmetadataViewPropsmatches the v2 shape. Nice and tidy.
520-535: Great assertions for dual sort callbacks.You verify trimmed internal
last_contacted_atwithASCand full external id withascending. That’s the way, fool.Consider adding a companion test that triggers the Action Bar sort dropdown and asserts both callbacks fire there too. I can draft it if you want.
src/elements/content-explorer/stories/MetadataView.stories.tsx (1)
68-68: Approve selection and sorting prop updates
I pity the fool who skips this check—grep confirms notableProps.isSelectAllEnabledorallowSorting:usages remain.src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx (1)
86-86: Story updates toisSelectionEnabled: thumbs up, sucka.Consistent with the new API; selection UX will render as expected.
Also applies to: 120-120, 189-189, 230-230
greg-in-a-box
force-pushed
the
sortingdropdown
branch
from
29f3398 to
6d1eb6d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
236-247: Don’t clobber consumer dropdown props, fool—merge ’em.Preserve existing sortDropdownProps while wiring your handler. Also good job placing handler above to avoid TDZ.
- sortDropdownProps: { - onSortChange: handleSortChange, - }, + sortDropdownProps: { + ...(actionBarProps?.sortDropdownProps ?? {}), + onSortChange: handleSortChange, + },
🧹 Nitpick comments (3)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
156-166: Filter sync is right, but ditch the O(n*m) scan, fool.Build a Set of trimmed column ids once, then filter fields in O(n). Faster, cleaner.
- const columnIds = newColumns.map(col => col.id); - fields = fields.filter((field: MetadataTemplateField) => { - // For metadata fields, check if the column ID matches the field key - // Column IDs for metadata fields are typically in format: metadata.template.fieldKey - return columnIds.some(columnId => { - const trimmedColumnId = trimMetadataFieldPrefix(columnId); - return trimmedColumnId === field.key; - }); - }); + const trimmedColumnIds = new Set(newColumns.map(col => trimMetadataFieldPrefix(col.id))); + // Only include fields that have corresponding visible columns + fields = fields.filter((field: MetadataTemplateField) => trimmedColumnIds.has(field.key));src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx (2)
139-166: DRY up repeated column scaffolding.You repeat the same Name/Industry column blocks across tests. Centralize with a small helper to keep tests tight and less error-prone.
Example helper to add per-test fields:
function makeColumns(extra: Array<{ id: string; textValue: string }>) { const base = [ { textValue: 'Name', id: 'item.name', type: 'string', allowsSorting: true, minWidth: 250, maxWidth: 250, isRowHeader: true }, ]; return [ ...base, ...extra.map(({ id, textValue }) => ({ textValue, id, type: 'string', allowsSorting: true, minWidth: 250, maxWidth: 250, })), ]; }Use per test, e.g., columns={makeColumns([{ id: 'role', textValue: 'Contact Role' }])}
Also applies to: 205-232, 270-297, 421-450, 486-521
334-338: Brittle counter assertion.‘All Filters 1’ hard-codes the count string. If UX changes spacing or locale, this breaks. Assert the chip state directly or check the specific filter chip badge instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP (Model Context Protocol) integration is disabled
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
package.json(2 hunks)src/elements/content-explorer/ContentExplorer.tsx(4 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(3 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(3 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(7 hunks)src/elements/content-explorer/stories/MetadataView.stories.tsx(1 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx
- src/elements/content-explorer/stories/MetadataView.stories.tsx
- package.json
- src/elements/content-explorer/tests/ContentExplorer.test.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-27T15:25:53.253Z
Learnt from: jpan-box
PR: box/box-ui-elements#4248
File: src/elements/content-explorer/MetadataViewContainer.tsx:30-44
Timestamp: 2025-08-27T15:25:53.253Z
Learning: In the Box API, there is an inconsistency where some endpoints require trimmed metadata field names (e.g., "industry") while others require the full field path (e.g., "metadata.enterprise_1515946.templateName.industry"). The trimMetadataFieldPrefix helper function in MetadataViewContainer.tsx correctly trims the field names for API endpoints that expect the shorter format.
Applied to files:
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
Applied to files:
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsxsrc/elements/content-explorer/MetadataViewContainer.tsx
📚 Learning: 2025-08-28T15:38:35.122Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
Applied to files:
src/elements/content-explorer/MetadataViewContainer.tsx
🧬 Code graph analysis (1)
src/elements/content-explorer/ContentExplorer.tsx (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
MetadataViewContainerProps(105-112)
⏰ 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). (2)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (5)
src/elements/content-explorer/MetadataViewContainer.tsx (2)
197-197: Good memo deps, sucka.Including newColumns in the deps keeps filters in lockstep with visible columns.
215-234: Sort bridge LGTM.Trims metadata prefix, maps to ASC/DESC for internal, and forwards external SortDescriptor. That’s the real deal, not just console noise.
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx (1)
662-745: Test mismatch: you included Industry but expect it missing. Fix it, fool.The test says columns only for Status and Price, but you also pass Industry. Either remove Industry from columns or change the expectation. Recommend removing it.
renderComponent({ metadataTemplate: template, columns: [ { textValue: 'Name', id: 'item.name', type: 'string', allowsSorting: true, minWidth: 250, maxWidth: 250, isRowHeader: true, }, - { - textValue: 'Industry', - id: 'industry', - type: 'string', - allowsSorting: true, - minWidth: 250, - maxWidth: 250, - }, { textValue: 'Status', id: 'status', type: 'string', allowsSorting: true, minWidth: 250, maxWidth: 250, }, { textValue: 'Price', id: 'price', type: 'string', allowsSorting: true, minWidth: 250, maxWidth: 250, }, ], });Likely an incorrect or invalid review comment.
src/elements/content-explorer/ContentExplorer.tsx (2)
407-424: Cloning the template before setState is the right move.Prevents sneaky mutations from upstream. Solid.
1614-1634: Selection flattening looks clean, sucka.Forwarding onSelectionChange and owning selectedKeys in state is correct, and you close the panel on empty selection. Nice.
greg-in-a-box
force-pushed
the
sortingdropdown
branch
from
6d1eb6d to
01d38e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/elements/content-explorer/ContentExplorer.tsx (1)
481-487: Normalize sortBy to string before building order_by.react-aria Key can be number. Box API expects string field keys. Don’t get caught slippin’, fool. Normalize once to avoid surprises.
Apply this diff:
@@ - if (isFeatureEnabled(features, 'contentExplorer.metadataViewV2')) { + const normalizedSortBy = typeof sortBy === 'string' ? sortBy : String(sortBy); + const sortFieldForMetadata = normalizedSortBy === FIELD_NAME ? FIELD_ITEM_NAME : normalizedSortBy; + if (isFeatureEnabled(features, 'contentExplorer.metadataViewV2')) { metadataQueryClone.order_by = [ { - // Default to the prefixed name field for metadata view v2 only, while not touching the default sortBy for other views. - field_key: sortBy === FIELD_NAME ? FIELD_ITEM_NAME : sortBy, + // Default to the prefixed name field for metadata view v2 only, while not touching the default sortBy for other views. + field_key: sortFieldForMetadata, direction: sortDirection, }, ]; @@ } else { metadataQueryClone.order_by = [ { - field_key: sortBy, + field_key: normalizedSortBy, direction: sortDirection, }, ];Also applies to: 497-503
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP (Model Context Protocol) integration is disabled
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
package.json(2 hunks)src/elements/content-explorer/ContentExplorer.tsx(4 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(3 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(2 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(7 hunks)src/elements/content-explorer/stories/MetadataView.stories.tsx(1 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/elements/content-explorer/stories/MetadataView.stories.tsx
- src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx
- package.json
- src/elements/content-explorer/tests/MetadataViewContainer.test.tsx
- src/elements/content-explorer/tests/ContentExplorer.test.tsx
- src/elements/content-explorer/MetadataViewContainer.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
⏰ 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). (2)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (5)
src/elements/content-explorer/ContentExplorer.tsx (5)
409-426: Cloning metadataTemplate to avoid accidental mutation. Right on.Deep clone before stashing in state prevents nasty side effects when children fiddle with the template. Clean and safe.
1616-1623: getMetadataViewProps: flatter selection wiring. Looks good.Passing selectedKeys + wrapping onSelectionChange at the top level is exactly what the doctor ordered.
145-148: Metadata selection fully centralized—no stale overrides
I pity the fool who tries to overrideselectedKeys—search across all TS/TSX files found zero instances of manualselectedKeysinmetadataViewPropsor legacytableProps.onSelectionChangenesting; callers and docs are in sync.
1869-1870: allowsSorting usage verified: All column configs consistently useallowsSorting—no strayallowSortingfound. I pity the fool who doubted it!
1626-1634: I pity the fool worried about'all'—all consumers guard it. Every downstream use checks=== 'all'before calling.sizeor.has, so no breakage will occur.Likely an incorrect or invalid review comment.
greg-in-a-box
force-pushed
the
sortingdropdown
branch
from
01d38e9 to
91525bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
239-253: I pity the fool who doesn't spread those props properly!You're not preserving any existing
sortDropdownPropsfromactionBarProps. If consumers pass additional sort dropdown configuration, you're gonna lose it!Apply this diff to preserve existing props:
const transformedActionBarProps = React.useMemo(() => { return { ...actionBarProps, initialFilterValues, onFilterSubmit: handleFilterSubmit, filterGroups, sortDropdownProps: { + ...(actionBarProps?.sortDropdownProps ?? {}), onSortChange: handleSortChange, }, predefinedFilterOptions: { [PredefinedFilterName.KeywordSearchFilterGroup]: { isDisabled: true }, [PredefinedFilterName.LocationFilterGroup]: { isDisabled: true }, }, }; }, [actionBarProps, initialFilterValues, handleFilterSubmit, handleSortChange, filterGroups]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP (Model Context Protocol) integration is disabled
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
package.json(2 hunks)src/elements/content-explorer/ContentExplorer.tsx(4 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(3 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(2 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(7 hunks)src/elements/content-explorer/stories/MetadataView.stories.tsx(1 hunks)src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/elements/content-explorer/stories/tests/MetadataView-visual.stories.tsx
- src/elements/content-explorer/stories/MetadataView.stories.tsx
- src/elements/content-explorer/tests/ContentExplorer.test.tsx
- package.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-28T15:38:35.122Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
Applied to files:
src/elements/content-explorer/MetadataViewContainer.tsx
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
Applied to files:
src/elements/content-explorer/MetadataViewContainer.tsxsrc/elements/content-explorer/ContentExplorer.tsxsrc/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx
📚 Learning: 2025-08-27T15:25:53.253Z
Learnt from: jpan-box
PR: box/box-ui-elements#4248
File: src/elements/content-explorer/MetadataViewContainer.tsx:30-44
Timestamp: 2025-08-27T15:25:53.253Z
Learning: In the Box API, there is an inconsistency where some endpoints require trimmed metadata field names (e.g., "industry") while others require the full field path (e.g., "metadata.enterprise_1515946.templateName.industry"). The trimMetadataFieldPrefix helper function in MetadataViewContainer.tsx correctly trims the field names for API endpoints that expect the shorter format.
Applied to files:
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx
🧬 Code graph analysis (1)
src/elements/content-explorer/ContentExplorer.tsx (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
MetadataViewContainerProps(107-114)
⏰ 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). (2)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (4)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
158-168: LGTM! Column-driven filtering is smart work, sucka!I appreciate the changes that sync fields with visible columns. The column-driven approach helps ensure filters only show up for fields that users can actually see in the table. That's good design, fool!
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx (1)
636-718: Good test coverage, sucka! Column-driven filtering working like it should!This new test properly validates that filters only show up when corresponding columns are provided. Smart way to ensure the UI stays clean and relevant!
src/elements/content-explorer/ContentExplorer.tsx (2)
409-409: Smart move cloning that template, fool!Good defensive programming! Cloning the metadata template prevents any accidental mutations from messing up the original data structure.
1616-1636: I see you flattened that API structure real good!Nice work moving
selectedKeysandonSelectionChangeto the top level instead of nesting them undertableProps. This makes the API cleaner and more intuitive for consumers. The empty selection check to close the side panel is a thoughtful UX touch too!
src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx
Outdated
Show resolved
Hide resolved
greg-in-a-box
force-pushed
the
sortingdropdown
branch
from
91525bc to
3569ea2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
239-254: Don’t clobber consumer props for the sort dropdown and predefined filters.Deep-merge to preserve caller options while still wiring your handler and forced disables. I pity the fool who overwrites customer props!
return { ...actionBarProps, initialFilterValues, onFilterSubmit: handleFilterSubmit, filterGroups, sortDropdownProps: { - onSortChange: handleSortChange, + ...(actionBarProps?.sortDropdownProps ?? {}), + onSortChange: handleSortChange, }, - predefinedFilterOptions: { - [PredefinedFilterName.KeywordSearchFilterGroup]: { isDisabled: true }, - [PredefinedFilterName.LocationFilterGroup]: { isDisabled: true }, - }, + predefinedFilterOptions: { + ...(actionBarProps?.predefinedFilterOptions ?? {}), + [PredefinedFilterName.KeywordSearchFilterGroup]: { + ...(actionBarProps?.predefinedFilterOptions?.[PredefinedFilterName.KeywordSearchFilterGroup] ?? {}), + isDisabled: true, + }, + [PredefinedFilterName.LocationFilterGroup]: { + ...(actionBarProps?.predefinedFilterOptions?.[PredefinedFilterName.LocationFilterGroup] ?? {}), + isDisabled: true, + }, + }, };
🧹 Nitpick comments (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
158-168: Speed it up, fool: pre-trim columns once and use a Set.Current code does O(N×M) scans and trims per field. Build a Set of trimmed IDs up-front for O(N+M) and cleaner intent.
- const columnIds = newColumns.map(col => col.id); - fields = fields.filter((field: MetadataTemplateField) => { - // For metadata fields, check if the column ID matches the field key - // Column IDs for metadata fields are typically in format: metadata.template.fieldKey - return columnIds.some(columnId => { - const trimmedColumnId = trimMetadataFieldPrefix(columnId); - return trimmedColumnId === field.key; - }); - }); + // Pre-trim once; Key may be string/number, normalize to string + const trimmedColumnIds = new Set( + newColumns.map(col => trimMetadataFieldPrefix(String(col.id))), + ); + // Keep only fields with a visible matching column + fields = fields.filter((field: MetadataTemplateField) => trimmedColumnIds.has(field.key));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP (Model Context Protocol) integration is disabled
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/elements/content-explorer/ContentExplorer.tsx(4 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(3 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(2 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/elements/content-explorer/tests/ContentExplorer.test.tsx
- src/elements/content-explorer/ContentExplorer.tsx
- src/elements/content-explorer/tests/MetadataViewContainer.test.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
📚 Learning: 2025-08-28T15:38:35.122Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
Applied to files:
src/elements/content-explorer/MetadataViewContainer.tsx
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
Applied to files:
src/elements/content-explorer/MetadataViewContainer.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). (2)
- GitHub Check: lint_test_build
- GitHub Check: Summary
greg-in-a-box
force-pushed
the
sortingdropdown
branch
from
3569ea2 to
86f3c38
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/elements/content-explorer/__tests__/ContentExplorer.test.tsx (1)
466-472: Nice:onMetadataFilterplumbed intometadataViewProps. Don’t leave it idle, sucka — add an assertion.Consider a small test that triggers a filter action and expects your mock to fire once.
src/elements/content-explorer/ContentExplorer.tsx (1)
1615-1635: Solid selection bridge with auto-closing side panel. I pity the fool who forgets to sync UI with selection!Minor nit: the inline handler is recreated every render. Consider a class field to stabilize identity and reduce child re-renders.
Apply within this range:
- return { - ...metadataViewProps, - selectedKeys: selectedItemIds, - onSelectionChange: (ids: Selection) => { - onSelectionChange?.(ids); - const isSelectionEmpty = ids !== 'all' && ids.size === 0; - this.setState({ - selectedItemIds: ids, - ...(isSelectionEmpty && { isMetadataSidePanelOpen: false }), - }); - }, - }; + return { + ...metadataViewProps, + selectedKeys: selectedItemIds, + onSelectionChange: this.handleMetadataSelectionChange, + };Add this method elsewhere in the class:
private handleMetadataSelectionChange = (ids: Selection) => { const { metadataViewProps } = this.props; metadataViewProps?.onSelectionChange?.(ids); const isSelectionEmpty = ids !== 'all' && (ids as Set<Key>).size === 0; this.setState({ selectedItemIds: ids, ...(isSelectionEmpty && { isMetadataSidePanelOpen: false }), }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP (Model Context Protocol) integration is disabled
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/elements/content-explorer/ContentExplorer.tsx(2 hunks)src/elements/content-explorer/MetadataViewContainer.tsx(3 hunks)src/elements/content-explorer/__tests__/ContentExplorer.test.tsx(2 hunks)src/elements/content-explorer/__tests__/MetadataViewContainer.test.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/elements/content-explorer/tests/MetadataViewContainer.test.tsx
- src/elements/content-explorer/MetadataViewContainer.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
📚 Learning: 2025-08-28T15:38:35.122Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataViewContainer.tsx:106-123
Timestamp: 2025-08-28T15:38:35.122Z
Learning: In the box/metadata-view Column interface used in MetadataViewContainer.tsx, the correct property name for enabling sorting is `allowsSorting`, not `allowSorting`. This is consistently used throughout the metadata-view related files in the codebase.
Applied to files:
src/elements/content-explorer/__tests__/ContentExplorer.test.tsx
📚 Learning: 2025-08-25T16:19:22.007Z
Learnt from: greg-in-a-box
PR: box/box-ui-elements#4235
File: src/elements/content-explorer/MetadataQueryBuilder.ts:103-110
Timestamp: 2025-08-25T16:19:22.007Z
Learning: In src/elements/content-explorer/MetadataQueryBuilder.ts, the getSelectFilter function has a special case that maps 'mimetype-filter' to 'item.extension' because mimetype-filter is a multi-select field but the actual database field is 'item.extension'. This mapping is necessary and should not be removed.
Applied to files:
src/elements/content-explorer/ContentExplorer.tsx
🧬 Code graph analysis (1)
src/elements/content-explorer/ContentExplorer.tsx (1)
src/elements/content-explorer/MetadataViewContainer.tsx (1)
MetadataViewContainerProps(107-114)
⏰ 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). (2)
- GitHub Check: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (2)
src/elements/content-explorer/__tests__/ContentExplorer.test.tsx (1)
446-464: All sorting flags uniformlyallowsSorting
Scan found zeroallowSortingusages, fool.src/elements/content-explorer/ContentExplorer.tsx (1)
145-148: Good API fence: own the selection state at the CE level.Omitting
selectedKeysfrom inboundmetadataViewPropsprevents external control conflicts. That’s clean.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
3 participants
Fixed the column only filter chip bug
updated metadata-view package with new selection props
enable sorting dropdown
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests