-
Notifications
You must be signed in to change notification settings - Fork 344
fix(blueprint): Bump blueprint #4205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change updates several Changes
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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsflow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (6)Learnt from: rafalmaksymiuk Learnt from: rafalmaksymiuk Learnt from: rafalmaksymiuk Learnt from: rafalmaksymiuk Learnt from: rafalmaksymiuk Learnt from: jpan-box 🪛 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)
✨ 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. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
1c129d3 to
86c1132
Compare
There was a problem hiding this 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$FlowFixMenow that we’re importing typed Blueprint iconsThe original suppress-comment was necessary for locally-defined icons lacking Flow types. The new
@box/blueprint-web-assetspackage 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 passingundefineddown as theuseV2Iconsflag
ICON_RENDERER[type](useV2Icons)will treatundefinedas 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 impactSwitching to
@box/item-iconcentralises 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 bloatDepending on how
@box/item-iconis 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 diffsFor 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
⛔ Files ignored due to path filters (1)
yarn.lockis 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_INFOnow rendersCheckmarkCircle, which visually communicates “success” rather than “information”. Meanwhile,InformationCircleis 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 storiesAdding 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-weband@box/blueprint-web-assetsare 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.jsonor open a follow-up issue so the team remembers to revisit the pin.Also applies to: 291-292
134-135: No deprecatedItemTypeIconprops found
A quick grep forItemTypeIcon.*(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.
86c1132 to
e834719
Compare
0b33e0e to
e031e2e
Compare
There was a problem hiding this 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 standardflow-typedstub metadata headerflow-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.xIncluding it:
- Makes the origin obvious to future maintainers.
- Lets the
flow-typedCLI 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
⛔ Files ignored due to path filters (1)
yarn.lockis 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 ignoredBiome flags the
declarekeyword as “TypeScript-only”. In Flow stubs (// @flowplusdeclare modulesyntax) this is perfectly valid, so no action required.
e031e2e to
33c4b0f
Compare
Summary by CodeRabbit
New Features
Chores