feat(sidebar-nav): update skills and docgen icons to BP (WEBAPP-42958) #4399
Conversation
|
|
WalkthroughAdds Flow exports for MagicWand and DocGen icons in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-09-03T18:10:42.760ZApplied to files:
📚 Learning: 2025-09-03T18:10:29.467ZApplied to files:
📚 Learning: 2025-06-17T13:30:02.172ZApplied to files:
📚 Learning: 2025-09-09T18:21:24.552ZApplied to files:
📚 Learning: 2025-06-25T13:09:54.538ZApplied to files:
📚 Learning: 2025-06-25T13:10:19.128ZApplied to files:
📚 Learning: 2025-08-12T18:04:17.698ZApplied to files:
📚 Learning: 2025-06-25T13:09:45.168ZApplied to files:
📚 Learning: 2025-09-03T18:30:44.447ZApplied to files:
🧬 Code graph analysis (1)src/elements/content-sidebar/SidebarNav.js (2)
🪛 Biome (2.1.2)src/elements/content-sidebar/SidebarNav.js[error] 98-98: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax. TypeScript only syntax (parse) [error] 109-109: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax. TypeScript only syntax (parse) flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js[error] 19-20: 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). (3)
🔇 Additional comments (5)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js(1 hunks)package.json(1 hunks)src/elements/content-sidebar/SidebarNav.js(4 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: tjuanitas
Repo: box/box-ui-elements PR: 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.
Learnt from: rafalmaksymiuk
Repo: box/box-ui-elements 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
Repo: box/box-ui-elements 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.
📚 Learning: 2025-06-25T13:09:54.538Z
Learnt from: rafalmaksymiuk
Repo: box/box-ui-elements 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.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-06-25T13:10:19.128Z
Learnt from: rafalmaksymiuk
Repo: box/box-ui-elements 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).
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-08-12T18:04:17.698Z
Learnt from: tjuanitas
Repo: box/box-ui-elements PR: 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:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.jspackage.json
📚 Learning: 2025-06-17T13:30:02.172Z
Learnt from: rafalmaksymiuk
Repo: box/box-ui-elements 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.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-09-16T14:53:03.538Z
Learnt from: bfoxx1906
Repo: box/box-ui-elements PR: 4275
File: src/components/form-elements/draft-js-mention-selector/index.js:4-4
Timestamp: 2025-09-16T14:53:03.538Z
Learning: The createMentionSelectorState export in src/components/form-elements/draft-js-mention-selector/index.js was confirmed to be internal-only within the box-ui-elements package, with no external references or documentation, making the breaking rename to createMentionTimestampSelectorState safe.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-06-25T13:09:45.168Z
Learnt from: rafalmaksymiuk
Repo: box/box-ui-elements 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.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-09-03T18:30:44.447Z
Learnt from: fpan225
Repo: box/box-ui-elements PR: 4239
File: src/elements/content-sidebar/SidebarPanels.js:0-0
Timestamp: 2025-09-03T18:30:44.447Z
Learning: In the CustomSidebarPanel type, the component field is required (React.ComponentType<any>), so runtime checks for component existence are unnecessary since Flow will catch missing components at compile time. User fpan225 prefers to rely on the type system rather than adding redundant runtime checks.
Applied to files:
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
📚 Learning: 2025-09-03T18:10:42.760Z
Learnt from: fpan225
Repo: box/box-ui-elements PR: 4239
File: src/elements/content-sidebar/SidebarNav.js:195-206
Timestamp: 2025-09-03T18:10:42.760Z
Learning: In src/elements/content-sidebar/SidebarNav.js, the maintainer wants users to be able to override data-* attributes (data-resin-target, data-target-id, data-testid) through navButtonProps for custom tabs. The spread of {...navButtonProps} should come after the data-* attributes to allow overriding, not before them.
Applied to files:
src/elements/content-sidebar/SidebarNav.js
🧬 Code graph analysis (1)
src/elements/content-sidebar/SidebarNav.js (2)
src/components/sidebar-toggle-button/SidebarToggleButton.js (1)
isPreviewModernizationEnabled(35-35)src/elements/content-sidebar/additional-tabs/AdditionalTabTooltip.js (1)
isPreviewModernizationEnabled(23-23)
🪛 Biome (2.1.2)
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js
[error] 15-16: The 'declare' modifier can only be used in TypeScript files.
(parse)
src/elements/content-sidebar/SidebarNav.js
[error] 98-98: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 109-109: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(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). (3)
- GitHub Check: lint_test_build
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Summary
🔇 Additional comments (4)
src/elements/content-sidebar/SidebarNav.js (3)
17-25: LGTM! New icon imports are properly organized.The imports for
MagicWandandDocGenicons from bothMediumandMediumFilledmodules are correctly structured and use appropriate aliases to avoid naming conflicts.
98-107: LGTM! MagicWandIconWrapper follows the established pattern.The wrapper correctly implements the conditional rendering logic: it uses the filled variant with blue color when active, the regular variant when inactive, and falls back to the legacy icon when preview modernization is disabled.
251-251: LGTM! Icon wrappers are correctly integrated.The new
MagicWandIconWrapperandDocGenIconWrappercomponents are properly used in place of the legacy icon components, with theisPreviewModernizationEnabledprop correctly passed through.Also applies to: 278-278
flow-typed/npm/@box/blueprint-web-assets_vx.x.x.js (1)
12-13: DocGen does not have a filled variant in blueprint-web-assets 4.88.2—current implementation is acceptable.The Flow types confirm that
DocGenis only exported from theMediummodule (line 13) and is absent fromMediumFilled(which includesMagicWand). TheDocGenIconWrapperimplementation correctly uses the sameDocGenicon for both active and inactive states, only changing the color. Since no filled variant exists, this implementation is consistent with the available assets.
kduncanhsu
force-pushed
the
WEBAPP-42958
branch
from
add268a to
abef76e
Compare
kduncanhsu
force-pushed
the
WEBAPP-42958
branch
from
9a5953b to
c94661e
Compare
kduncanhsu
force-pushed
the
WEBAPP-42958
branch
2 times, most recently
from
173dc74 to
8571e73
Compare
kduncanhsu
force-pushed
the
WEBAPP-42958
branch
from
8571e73 to
a2add9a
Compare
Merge Queue Status✅ The pull request has been merged at a2add9a This pull request spent 7 seconds in the queue, with no time running CI. Required conditions to merge
|
6 participants
JIRA ticket link
https://jira.inside-box.net/browse/WEBAPP-42958
Description
Update Skills and DocGen tabs in SidebarNav to use Blueprint icons
Screenshots
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.