Skip to content

Conversation

@greg-in-a-box
Copy link
Contributor

@greg-in-a-box greg-in-a-box commented Jul 28, 2025

Summary by CodeRabbit

  • New Features

    • Introduced updated icons for notifications, providing a refreshed visual experience.
    • Added new storybook examples showcasing notifications with the latest icon set.
    • Included Flow type declarations for improved static type checking of asset imports.
  • Chores

    • Updated several dependencies to newer versions.
    • Added a new dependency for shared types.
    • Adjusted testing configuration to support the new types package.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 28, 2025

Walkthrough

This change updates several @box package dependencies to newer versions, adds @box/types as a new dependency, and adjusts the Jest transform configuration to include it. Notification icon imports are updated to use new icons, and corresponding storybook stories are added for the new icon set. A Flow type declaration file for @box/blueprint-web-assets is introduced.

Changes

Cohort / File(s) Change Summary
Dependency Updates
package.json
Updated versions for @box/blueprint-web, @box/blueprint-web-assets, and @box/item-icon. Added @box/types to both dev and peer dependencies.
Jest Config Update
scripts/jest/jest.config.js
Added @box/types to the list of packages allowed for transformation in Jest.
Notification Icon Update
src/components/notification/Notification.js
Swapped icon imports and usage to new icons: InformationCircle, CheckmarkCircle, AlertCircle (replacing previous icons).
Notification Stories Update
src/components/notification/stories/Notification.stories.js
Added four new story exports (basicV2, infoV2, warnV2, errorV2) using the useV2Icons prop for Notification.
Flow Type Declarations
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
Added Flow type declarations for various icon and illustration modules within @box/blueprint-web-assets.

Sequence Diagram(s)

sequenceDiagram
  participant Storybook
  participant Notification
  participant Icons

  Storybook->>Notification: Render with (useV2Icons)
  Notification->>Icons: Import and select V2 icon components
  Notification-->>Storybook: Rendered notification with new icons
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

ready-to-merge

Suggested reviewers

  • tjuanitas
  • jsenar

Poem

A bundle of icons, shiny and new,
The Notification hops with a vibrant hue.
Dependencies fresh, the stories expand,
With Flow types to guide a rabbit’s hand.
In the garden of code, where updates bloom,
🐇 This patch brings spring to the notification room!

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e031e2e and 33c4b0f.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (1 hunks)
  • package.json (2 hunks)
  • scripts/jest/jest.config.js (1 hunks)
  • src/components/notification/Notification.js (2 hunks)
  • src/components/notification/stories/Notification.stories.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • scripts/jest/jest.config.js
  • package.json
  • src/components/notification/Notification.js
  • src/components/notification/stories/Notification.stories.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like `: Props` are valid Flow syntax, not TypeScript syntax.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4144
File: src/elements/content-sidebar/versions/VersionsList.js:24-33
Timestamp: 2025-06-17T13:30:02.172Z
Learning: In the box-ui-elements codebase, Flow components use .flow.js type definition files, not TypeScript .ts files. The InternalSidebarNavigation type is a union type where different variants may have different properties like versionId, and proper type safety is ensured through conditional checks in methods like getSelectedVersionId.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4160
File: src/elements/content-sidebar/SidebarToggle.js:11-11
Timestamp: 2025-06-25T13:10:19.128Z
Learning: In the box-ui-elements codebase, Flow files support `import type` syntax for type-only imports. The `import type` statement is valid Flow syntax, not just TypeScript, and should not be flagged as an error when used in Flow files (indicated by @flow comments).
Learnt from: jpan-box
PR: box/box-ui-elements#4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4160
File: src/elements/content-sidebar/SidebarToggle.js:13-19
Timestamp: 2025-06-25T13:09:45.168Z
Learning: Files with `@flow` or `@flow strict` comments at the top use Flow type syntax, not TypeScript. Flow type definitions like `type Props = { ... }` and type imports like `type { RouterHistory }` are valid Flow syntax and should not be flagged as TypeScript-only features.
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (6)

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like : Props are valid Flow syntax, not TypeScript syntax.

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:11-11
Timestamp: 2025-06-25T13:10:19.128Z
Learning: In the box-ui-elements codebase, Flow files support import type syntax for type-only imports. The import type statement is valid Flow syntax, not just TypeScript, and should not be flagged as an error when used in Flow files (indicated by @flow comments).

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:13-19
Timestamp: 2025-06-25T13:09:45.168Z
Learning: Files with @flow or @flow strict comments at the top use Flow type syntax, not TypeScript. Flow type definitions like type Props = { ... } and type imports like type { RouterHistory } are valid Flow syntax and should not be flagged as TypeScript-only features.

Learnt from: rafalmaksymiuk
PR: #4144
File: src/elements/content-sidebar/versions/VersionsList.js:24-33
Timestamp: 2025-06-17T13:30:02.172Z
Learning: In the box-ui-elements codebase, Flow components use .flow.js type definition files, not TypeScript .ts files. The InternalSidebarNavigation type is a union type where different variants may have different properties like versionId, and proper type safety is ensured through conditional checks in methods like getSelectedVersionId.

Learnt from: rafalmaksymiuk
PR: #4156
File: src/elements/content-sidebar/SidebarNavButton.js:14-14
Timestamp: 2025-06-24T18:35:01.363Z
Learning: The import type syntax is valid in Flow, not just TypeScript. Flow supports import type { Type } from 'module' for importing types, even in .js files with @flow pragma.

Learnt from: jpan-box
PR: #4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.

🪛 Biome (2.1.2)
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js

[error] 3-4: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 13-14: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 22-23: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 36-37: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 41-42: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 50-51: The 'declare' modifier can only be used in TypeScript files.

(parse)

⏰ 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
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@greg-in-a-box greg-in-a-box force-pushed the bp-upgrade-1 branch 2 times, most recently from 1c129d3 to 86c1132 Compare July 29, 2025 00:00
@greg-in-a-box greg-in-a-box marked this pull request as ready for review July 29, 2025 00:01
@greg-in-a-box greg-in-a-box requested a review from a team as a code owner July 29, 2025 00:01
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (5)
src/components/notification/Notification.js (2)

7-13: Consider removing the $FlowFixMe now that we’re importing typed Blueprint icons

The original suppress-comment was necessary for locally-defined icons lacking Flow types. The new @box/blueprint-web-assets package ships generated Flow definitions, so the suppression may no longer be needed. Dropping it will let Flow catch any future typing errors.


115-120: Avoid passing undefined down as the useV2Icons flag

ICON_RENDERER[type](useV2Icons) will treat undefined as falsey, but explicit boolean coercion improves readability and prevents surprises when the prop becomes tri-state in the future:

-const iconRenderer = ICON_RENDERER[type](useV2Icons);
+const iconRenderer = ICON_RENDERER[type](!!useV2Icons);
src/elements/common/item-list/ItemList.tsx (1)

8-9: Import path looks correct — verify tree-shaking size impact

Switching to @box/item-icon centralises the component, but the package currently ships all icon SVGs. If bundle size is a concern, consider importing the specific sub-entry (e.g. @box/item-icon/dist/ItemTypeIcon) once the library exposes it.

src/elements/common/item-grid/ItemGrid.tsx (1)

6-9: Same note as ItemList regarding potential bundle bloat

Depending on how @box/item-icon is built, importing from the package root may pull in unused icons. Keep an eye on bundle analyzer output after this change.

scripts/jest/jest.config.js (1)

28-30: Maintain alphabetical order in the allow-list for easier diffs

For long regex allow-lists, keeping package names sorted (@box/blueprint-web, @box/cldr-data, …, @box/tree, @box/types) helps future maintainers spot duplicates or omissions more quickly.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 435c75a and 86c1132.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (10)
  • package.json (2 hunks)
  • scripts/jest/jest.config.js (1 hunks)
  • src/components/notification/Notification.js (2 hunks)
  • src/components/notification/stories/Notification.stories.js (1 hunks)
  • src/elements/common/item-grid/ItemGrid.tsx (1 hunks)
  • src/elements/common/item-list/ItemList.tsx (1 hunks)
  • src/elements/common/item/ItemTypeIcon.tsx (0 hunks)
  • src/elements/common/item/__tests__/ItemTypeIcon.test.tsx (0 hunks)
  • src/elements/common/item/index.ts (0 hunks)
  • src/elements/common/modal.scss (0 hunks)
💤 Files with no reviewable changes (4)
  • src/elements/common/item/index.ts
  • src/elements/common/modal.scss
  • src/elements/common/item/tests/ItemTypeIcon.test.tsx
  • src/elements/common/item/ItemTypeIcon.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: jpan-box
PR: box/box-ui-elements#4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.
Learnt from: tjuanitas
PR: box/box-ui-elements#4126
File: .storybook/reactIntl.ts:4-7
Timestamp: 2025-06-17T15:39:16.739Z
Learning: In box-ui-elements, the team prefers using the spread operator in reduce functions to maintain immutability rather than mutating objects, even if it has a minor performance cost. This reflects their preference for functional programming principles and code safety over micro-optimizations.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like `: Props` are valid Flow syntax, not TypeScript syntax.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4144
File: src/elements/content-sidebar/versions/VersionsList.js:24-33
Timestamp: 2025-06-17T13:30:02.172Z
Learning: In the box-ui-elements codebase, Flow components use .flow.js type definition files, not TypeScript .ts files. The InternalSidebarNavigation type is a union type where different variants may have different properties like versionId, and proper type safety is ensured through conditional checks in methods like getSelectedVersionId.
scripts/jest/jest.config.js (4)

Learnt from: jpan-box
PR: #4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:11-11
Timestamp: 2025-06-25T13:10:19.128Z
Learning: In the box-ui-elements codebase, Flow files support import type syntax for type-only imports. The import type statement is valid Flow syntax, not just TypeScript, and should not be flagged as an error when used in Flow files (indicated by @flow comments).

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like : Props are valid Flow syntax, not TypeScript syntax.

Learnt from: tjuanitas
PR: #4126
File: scripts/webpack.config.js:72-76
Timestamp: 2025-06-17T15:21:36.180Z
Learning: The Box UI Elements project does not run webpack builds on Windows machines, so Windows path separator compatibility is not a concern for their build scripts.

src/elements/common/item-list/ItemList.tsx (3)

Learnt from: jpan-box
PR: #4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:11-11
Timestamp: 2025-06-25T13:10:19.128Z
Learning: In the box-ui-elements codebase, Flow files support import type syntax for type-only imports. The import type statement is valid Flow syntax, not just TypeScript, and should not be flagged as an error when used in Flow files (indicated by @flow comments).

Learnt from: rafalmaksymiuk
PR: #4144
File: src/elements/content-sidebar/versions/VersionsList.js:24-33
Timestamp: 2025-06-17T13:30:02.172Z
Learning: In the box-ui-elements codebase, Flow components use .flow.js type definition files, not TypeScript .ts files. The InternalSidebarNavigation type is a union type where different variants may have different properties like versionId, and proper type safety is ensured through conditional checks in methods like getSelectedVersionId.

src/elements/common/item-grid/ItemGrid.tsx (2)

Learnt from: jpan-box
PR: #4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:11-11
Timestamp: 2025-06-25T13:10:19.128Z
Learning: In the box-ui-elements codebase, Flow files support import type syntax for type-only imports. The import type statement is valid Flow syntax, not just TypeScript, and should not be flagged as an error when used in Flow files (indicated by @flow comments).

src/components/notification/Notification.js (3)

Learnt from: jpan-box
PR: #4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.

Learnt from: rafalmaksymiuk
PR: #4144
File: src/elements/content-sidebar/versions/VersionsList.js:24-33
Timestamp: 2025-06-17T13:30:02.172Z
Learning: In the box-ui-elements codebase, Flow components use .flow.js type definition files, not TypeScript .ts files. The InternalSidebarNavigation type is a union type where different variants may have different properties like versionId, and proper type safety is ensured through conditional checks in methods like getSelectedVersionId.

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like : Props are valid Flow syntax, not TypeScript syntax.

package.json (3)

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like : Props are valid Flow syntax, not TypeScript syntax.

Learnt from: tjuanitas
PR: #4126
File: scripts/webpack.config.js:72-76
Timestamp: 2025-06-17T15:21:36.180Z
Learning: The Box UI Elements project does not run webpack builds on Windows machines, so Windows path separator compatibility is not a concern for their build scripts.

Learnt from: jpan-box
PR: #4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.

src/components/notification/stories/Notification.stories.js (1)

Learnt from: jpan-box
PR: #4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.

🧬 Code Graph Analysis (1)
src/elements/common/item-list/ItemList.tsx (1)
src/features/unified-share-modal/utils/item.js (1)
  • convertToBoxItem (14-47)
⏰ 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/components/notification/Notification.js (1)

42-47: Double-check icon to notification-type mapping

TYPE_INFO now renders CheckmarkCircle, which visually communicates “success” rather than “information”. Meanwhile, InformationCircle is assigned to the default (black) variant. Please confirm with design that this swap is intentional; if not, the two icons should likely be exchanged.

src/components/notification/stories/Notification.stories.js (1)

43-74: 👍 Great addition of V2 stories

Adding separate stories for the V2 icon variant provides clear visual regression coverage without disturbing existing stories.

package.json (2)

127-128: Pinning removes auto-patches – confirm this is a short-term workaround

@box/blueprint-web and @box/blueprint-web-assets are now hard-pinned (no ^).
This freezes us on 12.43.0 / 4.61.5 and we will silently miss any future patch or security release until the pin is lifted.

If the intent is purely to avoid current regressions (per earlier comment), add a TODO in package.json or open a follow-up issue so the team remembers to revisit the pin.

Also applies to: 291-292


134-135: No deprecated ItemTypeIcon props found
A quick grep for ItemTypeIcon.*(size|color|badge) returned no hits, indicating all outdated prop references have been removed.

• If you have any custom CSS overrides for ItemTypeIcon, verify they still work with the new API.
• Run your Storybook visual-diff suite to confirm there are no regressions.

jpan-box
jpan-box previously approved these changes Jul 29, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (1)

1-3: Missing standard flow-typed stub metadata header

flow-typed stubs usually start with a generated-signature header, e.g.

// flow-typed signature: <<STUB>>/@box/blueprint-web-assets_v4.x.x/flow_v0.xx.x
// flow-typed version: <<STUB>>/@box/blueprint-web-assets_v4.x.x/flow_v0.xx.x

Including it:

  1. Makes the origin obvious to future maintainers.
  2. Lets the flow-typed CLI recognise and manage the file (e.g. during upgrades).

Not critical for type-checking, but worth adding for consistency.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b33e0e and e031e2e.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (6)
  • flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (1 hunks)
  • package.json (2 hunks)
  • scripts/jest/jest.config.js (1 hunks)
  • src/components/notification/Notification.js (2 hunks)
  • src/components/notification/stories/Notification.stories.js (1 hunks)
  • src/elements/common/modal.scss (0 hunks)
💤 Files with no reviewable changes (1)
  • src/elements/common/modal.scss
✅ Files skipped from review due to trivial changes (1)
  • scripts/jest/jest.config.js
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/components/notification/Notification.js
  • src/components/notification/stories/Notification.stories.js
  • package.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like `: Props` are valid Flow syntax, not TypeScript syntax.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4144
File: src/elements/content-sidebar/versions/VersionsList.js:24-33
Timestamp: 2025-06-17T13:30:02.172Z
Learning: In the box-ui-elements codebase, Flow components use .flow.js type definition files, not TypeScript .ts files. The InternalSidebarNavigation type is a union type where different variants may have different properties like versionId, and proper type safety is ensured through conditional checks in methods like getSelectedVersionId.
Learnt from: jpan-box
PR: box/box-ui-elements#4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4160
File: src/elements/content-sidebar/SidebarToggle.js:11-11
Timestamp: 2025-06-25T13:10:19.128Z
Learning: In the box-ui-elements codebase, Flow files support `import type` syntax for type-only imports. The `import type` statement is valid Flow syntax, not just TypeScript, and should not be flagged as an error when used in Flow files (indicated by @flow comments).
Learnt from: rafalmaksymiuk
PR: box/box-ui-elements#4160
File: src/elements/content-sidebar/SidebarToggle.js:13-19
Timestamp: 2025-06-25T13:09:45.168Z
Learning: Files with `@flow` or `@flow strict` comments at the top use Flow type syntax, not TypeScript. Flow type definitions like `type Props = { ... }` and type imports like `type { RouterHistory }` are valid Flow syntax and should not be flagged as TypeScript-only features.
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (6)

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:21-27
Timestamp: 2025-06-25T13:09:54.538Z
Learning: The box-ui-elements project uses Flow for type annotations in JavaScript files, as indicated by @flow directives in file headers. Type annotations like : Props are valid Flow syntax, not TypeScript syntax.

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:11-11
Timestamp: 2025-06-25T13:10:19.128Z
Learning: In the box-ui-elements codebase, Flow files support import type syntax for type-only imports. The import type statement is valid Flow syntax, not just TypeScript, and should not be flagged as an error when used in Flow files (indicated by @flow comments).

Learnt from: rafalmaksymiuk
PR: #4160
File: src/elements/content-sidebar/SidebarToggle.js:13-19
Timestamp: 2025-06-25T13:09:45.168Z
Learning: Files with @flow or @flow strict comments at the top use Flow type syntax, not TypeScript. Flow type definitions like type Props = { ... } and type imports like type { RouterHistory } are valid Flow syntax and should not be flagged as TypeScript-only features.

Learnt from: rafalmaksymiuk
PR: #4144
File: src/elements/content-sidebar/versions/VersionsList.js:24-33
Timestamp: 2025-06-17T13:30:02.172Z
Learning: In the box-ui-elements codebase, Flow components use .flow.js type definition files, not TypeScript .ts files. The InternalSidebarNavigation type is a union type where different variants may have different properties like versionId, and proper type safety is ensured through conditional checks in methods like getSelectedVersionId.

Learnt from: rafalmaksymiuk
PR: #4156
File: src/elements/content-sidebar/SidebarNavButton.js:14-14
Timestamp: 2025-06-24T18:35:01.363Z
Learning: The import type syntax is valid in Flow, not just TypeScript. Flow supports import type { Type } from 'module' for importing types, even in .js files with @flow pragma.

Learnt from: jpan-box
PR: #4166
File: src/elements/content-sidebar/SidebarNav.js:126-126
Timestamp: 2025-07-11T14:43:02.677Z
Learning: In the box-ui-elements repository, there's a file-type-based pattern for internationalization: TypeScript files (.tsx) predominantly use the modern useIntl hook (41 vs 15 files), while JavaScript files (.js) predominantly use the legacy injectIntl HOC (64 vs 5 files). New TypeScript components should use useIntl, while existing JavaScript components typically continue using injectIntl for consistency.

🪛 Biome (2.1.2)
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js

[error] 3-4: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 13-14: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 22-23: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 36-37: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 41-42: The 'declare' modifier can only be used in TypeScript files.

(parse)


[error] 50-51: The 'declare' modifier can only be used in TypeScript files.

(parse)

⏰ 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 (1)
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (1)

1-60: Static-analysis false positives can be safely ignored

Biome flags the declare keyword as “TypeScript-only”. In Flow stubs (// @flow plus declare module syntax) this is perfectly valid, so no action required.

jpan-box
jpan-box previously approved these changes Jul 29, 2025
@mergify mergify bot merged commit 5e7f4b2 into box:master Jul 29, 2025
7 checks passed
@greg-in-a-box greg-in-a-box deleted the bp-upgrade-1 branch July 29, 2025 19:45
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.

4 participants