fix: upgrade packadges #1256
fix: upgrade packadges #1256
Conversation
- Fix dark mode blink by adding color-scheme meta tag and explicit colors - Fix grid layout overflow issues with proper constraints and positioning - Improve color picker styling to match WDS design system - Replace @apache-arrow/ts with precompiled apache-arrow package - Remove code comments from eslint.config.js Fixes: - Dark mode flash on page load - Sidebar overflow causing content cutoff - Textarea switching to dark mode - Color picker missing hover/focus states Changes: - src/ui/index.html: Add color-scheme meta and form element styles - src/ui/src/builder/BuilderApp.vue: Fix grid layout constraints - src/ui/src/builder/sidebar/BuilderSidebar.vue: Add width constraints - src/ui/src/builder/settings/BuilderFieldsColor.vue: Enhance color picker - src/ui/src/wds/WdsTextareaInput.vue: Add explicit background color - src/ui/package.json: Replace @apache-arrow/ts with apache-arrow - src/ui/src/components/core/content/CoreDataframe.vue: Update imports - src/ui/eslint.config.js: Remove code comments
📝 WalkthroughWalkthroughReplaces the UI CommonJS ESLint config with a new eslint.config.js, upgrades many UI dependencies and scripts, adds a light color-scheme meta and visual styling updates, refactors builder layout/wrappers, hardens annotation color generation with fallbacks, and applies assorted typing/formatting/ui style tweaks across the UI package. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
HackerOne Code Security Review🟢 Scan Complete: 2 Issue(s) Here's how the code changes were interpreted and info about the tools used for scanning. 📖 Summary of ChangesThe project underwent a comprehensive ESLint configuration update, transitioning from a traditional to a flat config system. Several UI components received styling refinements, including grid layouts, sidebar properties, and input element backgrounds. Dependency versions were upgraded, and meta tags for color scheme were introduced. The changes appear to focus on improving code quality, styling consistency, and visual presentation.
ℹ️ Issues DetectedNOTE: These may not require action! Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem. How will I know if something is a problem?
🧰 Analysis tools
⏱️ Latest scan covered changes up to commit f95d798 (latest) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/ui/eslint.config.js (1)
1-158: Consider using theglobalspackage for browser globals.The manual declaration of 78+ browser globals (lines 41-118) is error-prone and difficult to maintain. Consider using the
globalspackage which provides curated sets:import globals from "globals"; // Then in languageOptions: languageOptions: { parser: vueParser, parserOptions: { parser: tsparser, ecmaVersion: 2020, sourceType: "module", }, globals: { ...globals.browser, ...globals.node, }, }Otherwise, the migration to ESLint flat config looks good with appropriate presets, plugins, and rules.
src/ui/package.json (1)
72-72: Consider pinningvolar-service-veturto a specific version.Using
"*"as a version specifier accepts any version, which may lead to unpredictable behavior if breaking changes are introduced in future releases. Consider specifying a version range or at minimum a major version constraint (e.g.,"^1.0.0").
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
src/ui/.eslintrc.cjssrc/ui/eslint.config.jssrc/ui/index.htmlsrc/ui/package.jsonsrc/ui/src/builder/BuilderApp.vuesrc/ui/src/builder/settings/BuilderFieldsColor.vuesrc/ui/src/builder/sidebar/BuilderSidebar.vuesrc/ui/src/components/core/input/CoreTextareaInput.vuesrc/ui/src/wds/WdsTextareaInput.vuesrc/ui/vite.config.custom.ts
💤 Files with no reviewable changes (1)
- src/ui/.eslintrc.cjs
⏰ 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). (8)
- GitHub Check: tests (webkit)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: build (3.13)
- GitHub Check: build (3.9)
🔇 Additional comments (15)
src/ui/src/components/core/input/CoreTextareaInput.vue (1)
125-125: LGTM: White background enforces light color scheme.The explicit white background aligns with the light color-scheme enforcement across the UI.
src/ui/src/wds/WdsTextareaInput.vue (1)
64-64: LGTM: Consistent white background styling.This matches the styling update in CoreTextareaInput.vue and enforces the light color scheme.
src/ui/index.html (2)
7-7: LGTM: Color-scheme meta tag enforces light mode.The meta tag properly declares the light color scheme at the document level.
21-38: LGTM: Comprehensive light color scheme enforcement.The explicit color-scheme declarations, white background, black text, and form element styling effectively prevent dark mode from interfering with the UI. The font-family declaration with the layout shift comment is appropriate.
src/ui/src/builder/settings/BuilderFieldsColor.vue (1)
126-172: LGTM: Enhanced color picker styling with good UX.The styling improvements include:
- Proper container layout
- Interactive states (hover, focus) with visual feedback
- Browser-specific customization for webkit and moz
- Consistent white background and light color-scheme
These changes improve the user experience without affecting functionality.
src/ui/vite.config.custom.ts (1)
23-23: LGTM: Proper convention for unused parameters.Prefixing unused parameters with underscore is a recognized convention that clearly signals intent and can help avoid linting warnings.
src/ui/src/builder/BuilderApp.vue (3)
539-542: LGTM: Grid overflow prevention.The
minmax(0, 1fr)prevents the grid from overflowing its container, andoverflow: hiddenon the mainGrid properly contains content. These are good practices for grid layouts.
552-565: LGTM: Sidebar sizing constraints improve predictability.The explicit sizing constraints, border adjustments, and overflow handling make the sidebar layout more predictable and maintainable.
574-586: Clarify the full-width implementation approach.The CSS classes
.builderMainWrapperand.builderMainWrapper--fullWidthare defined but not applied to any template element. The actual full-width behavior for preview, vault, and journal modes uses themainGrid--fullWidthclass binding on theShareResizeVerticalcomponent (lines 18-22), which has no corresponding CSS definition. Either the builderMainWrapper classes should be applied to the appropriate template element, or the unused CSS definitions should be removed to avoid confusion. Verify which approach is intended and ensure it's properly implemented.Likely an incorrect or invalid review comment.
src/ui/src/builder/sidebar/BuilderSidebar.vue (2)
225-228: LGTM: Sidebar sizing constraints aligned with parent layout.The explicit sizing constraints (max-width: 288px) align with the BuilderApp.vue sidebar specifications and prevent layout overflow issues.
275-279: LGTM: Pane styling consistency.The white background and tight sizing constraints (240px) ensure consistent pane rendering and prevent overflow issues.
src/ui/package.json (4)
34-34: Verify Vue version compatibility with VueUse 14.x.From v14.0, VueUse requires Vue v3.5+. The specified Vue version
^3.5.26satisfies this requirement.Also applies to: 52-52
70-70: Vite 7.x requires updated Node.js version.Vite now requires Node.js 20.19+, 22.12+. We have dropped Node.js 18, now that it has reached its EOL at the end of April 2025.
Ensure your CI/CD pipelines and local development environments meet these Node.js version requirements.
37-37: No action needed. The migration from@apache-arrow/tstoapache-arrowhas been completed successfully. No old package imports remain in the codebase, and the new package is properly imported with the correct API (Table type and tableFromIPC function) in CoreDataframe.vue.
9-10: Lint scripts correctly updated for ESLint 9.x flat config.The simplified ESLint commands align with the ESLint 9.39.2 flat config format. The corresponding
eslint.config.jsis already in place and properly configured, with the deprecated.eslintrc.cjsalready removed. The migration is complete.
|
✅ Graham C reviewed all the included code changes and associated automation findings and determined that there were no immediately actionable security flaws. Note that they will continue to be notified of any new commits or comments and follow up as needed throughout the duration of this pull request's lifecycle.
Reviewed with ❤️ by PullRequest |
- Remove non-existent @typescript-eslint/ban-types disable comment - Fix unnecessary escape character in regex pattern - Replace single quotes with double quotes in builderEditorWorker.ts - Replace any type with unknown in builderEditorWorker.ts
- Add error handling and fallbacks to ensure getAnnotationBgColor always returns valid color - Improve generateColorCss to use rgb format for better browser compatibility - Add getAnnotationStyle helper to ensure style object always has backgroundColor - Update test to check both HTML attribute and inline style property (Vue applies styles directly to element.style) - Fix snapshot to match actual rendered output
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/ui/src/components/core/content/CoreAnnotatedText.vue (2)
222-249: Consider extracting repeated fallback color logic.The fallback pattern
fields.referenceColor.value || WdsColor.Blue5 || "#0066cc"appears multiple times acrossgetAnnotationBgColor,getAnnotationStyle, andgenerateColor.🔎 Optional refactor to reduce duplication
+const getFallbackColor = () => fields.referenceColor.value || WdsColor.Blue5 || "#0066cc"; + function getAnnotationBgColor(content: AnnotatedTextElementArray): string { try { if (content[2] && typeof content[2] === "string" && content[2].trim()) { return content[2]; } if (content[1] && typeof content[1] === "string") { const generatedColor = generateColor(content[1]); if (generatedColor && typeof generatedColor === "string" && generatedColor.trim()) { return generatedColor; } } } catch (error) { // Fall through to fallback } - // Ensure we always return a valid color string - const fallbackColor = fields.referenceColor.value || WdsColor.Blue5 || "#0066cc"; - return typeof fallbackColor === "string" ? fallbackColor : "#0066cc"; + return getFallbackColor(); }
245-248: RedundantString()wrapper.
finalColoris already guaranteed to be a string at this point (either from the validatedbgColoror from the string literal fallback), soString(finalColor)is unnecessary.- const finalColor = (bgColor && typeof bgColor === "string" && bgColor.trim()) - ? bgColor - : (fields.referenceColor.value || WdsColor.Blue5 || "#0066cc"); - return { backgroundColor: String(finalColor) }; + const finalColor = (bgColor && typeof bgColor === "string" && bgColor.trim()) + ? bgColor + : (fields.referenceColor.value || WdsColor.Blue5 || "#0066cc"); + return { backgroundColor: finalColor };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
src/ui/src/components/core/content/__snapshots__/CoreAnnotatedText.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (17)
src/ui/src/builder/builderEditorWorker.tssrc/ui/src/builder/builderManager.tssrc/ui/src/builder/settings/BuilderFieldsAlign.vuesrc/ui/src/builder/settings/BuilderSettingsProperties.vuesrc/ui/src/builder/useComponentActions.tssrc/ui/src/builder/useDragDropComponent.tssrc/ui/src/components/core/content/CoreAnnotatedText.spec.tssrc/ui/src/components/core/content/CoreAnnotatedText.vuesrc/ui/src/components/core/content/CoreDataframe/useDataframeValueBroker.tssrc/ui/src/constants/retry.tssrc/ui/src/constants/validators.tssrc/ui/src/core/index.tssrc/ui/src/core/launchDarklyClient.tssrc/ui/src/main.tssrc/ui/src/stories/fakeCore.tssrc/ui/src/utils/img.tssrc/ui/src/writerApi.ts
💤 Files with no reviewable changes (1)
- src/ui/src/constants/retry.ts
✅ Files skipped from review due to trivial changes (7)
- src/ui/src/core/launchDarklyClient.ts
- src/ui/src/components/core/content/CoreDataframe/useDataframeValueBroker.ts
- src/ui/src/writerApi.ts
- src/ui/src/builder/settings/BuilderSettingsProperties.vue
- src/ui/src/stories/fakeCore.ts
- src/ui/src/main.ts
- src/ui/src/builder/builderManager.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ui/src/builder/useDragDropComponent.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/ui/src/utils/img.ts (1)
src/ui/src/composables/useLogger.ts (1)
ILogger(3-3)
src/ui/src/builder/useComponentActions.ts (1)
src/ui/src/writerTypes.ts (1)
Component(25-44)
⏰ 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). (8)
- GitHub Check: build (3.9)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: tests (webkit)
- GitHub Check: build (3.11)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
- GitHub Check: build (3.13)
🔇 Additional comments (10)
src/ui/src/utils/img.ts (1)
6-89: LGTM! Formatting improvements from ESLint config update.The changes are purely formatting adjustments (indentation, line breaks, and whitespace) resulting from the ESLint configuration upgrade. No logic, error handling, or function behavior has been modified. The reformatted code maintains proper resource cleanup and error handling while improving consistency.
src/ui/src/builder/useComponentActions.ts (4)
251-269: LGTM!The parentheses clarify operator precedence, and the nullish coalescing provides a safe fallback. When
index > 0, the accumulator is guaranteed to have at least one element from the previous iteration, soacc.at(-1)will be defined.
1142-1144: LGTM!Formatting-only change with no semantic impact.
1183-1185: LGTM!Formatting-only change with no semantic impact.
1198-1200: LGTM!Formatting-only change with no semantic impact.
src/ui/src/constants/validators.ts (1)
180-212: LGTM! Formatting improvements enhance readability.The multi-line formatting of the
validatorChatBotMessageschema improves readability without introducing any semantic changes. The explicit structure of theimage_urlvalidation (withformatandpatternon separate lines) makes the validation constraints clearer.src/ui/src/builder/builderEditorWorker.ts (1)
10-10: Good type safety improvement.Changing the unused parameter type from
anytounknownis a best practice that prevents accidental usage without proper type checking.src/ui/src/builder/settings/BuilderFieldsAlign.vue (1)
66-76: LGTM! Suppressions correctly removed as rule is not enabled.The removal of ESLint suppressions is correct—the
no-duplicate-enum-valuesrule is not configured in the new flat ESLint config (src/ui/eslint.config.js). The duplicate values inSubModeenum (horizontal and vertical alignment using the same CSS values) are intentional and won't be flagged.src/ui/src/components/core/content/CoreAnnotatedText.vue (2)
185-210: Good defensive error handling for color generation.Wrapping the chroma operations in try/catch with fallbacks is a sensible approach to ensure robustness during package upgrades.
251-282: LGTM!The added fallbacks throughout
generateColorensure a valid color string is always returned, preventing potential rendering issues from chroma-js operations.
- Use globals package for ESLint browser/node globals instead of manual declaration - Pin volar-service-vetur to ^0.0.64 instead of wildcard version - Add missing globals (MediaStreamConstraints, RequestInit) not in standard set
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/ui/eslint.config.js (2)
35-49: Consider updatingecmaVersionto a more recent version.The
ecmaVersionis set to 2020. Modern projects typically use 2022 or "latest" to support newer JavaScript features like top-level await, private class fields, and other recent additions.🔎 Suggested update
languageOptions: { parser: vueParser, parserOptions: { parser: tsparser, - ecmaVersion: 2020, + ecmaVersion: "latest", sourceType: "module", },
83-83: Reconsider the strictness of theno-consolerule.Setting
no-consoleto "error" will block all console statements, including potentially useful ones likeconsole.errorandconsole.warnthat are often used for debugging and error tracking in development. Consider using "warn" instead, or configure it to allow specific console methods.🔎 Suggested alternatives
Option 1: Downgrade to warning
- "no-console": "error", + "no-console": "warn",Option 2: Allow error and warn
- "no-console": "error", + "no-console": ["error", { allow: ["warn", "error"] }],
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
src/ui/eslint.config.jssrc/ui/package.json
⏰ 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). (8)
- GitHub Check: tests (webkit)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: build (3.9)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
🔇 Additional comments (2)
src/ui/package.json (2)
9-10: LGTM - Lint scripts are simplified and clear.The simplified lint commands align well with the new ESLint flat config approach. Using
eslint . --fixfor development andeslint .for CI is a clean, straightforward pattern.
20-73: The major version upgrades are already properly configured for ESLint 9.ESLint 9 flat config is correctly implemented in
eslint.config.jswith all plugins properly configured:
@typescript-eslint/eslint-plugin(8.50.1)eslint-plugin-vue(10.6.2) with correct flat configeslint-plugin-prettier(5.5.4)Package versions are compatible with each other, and there are no evidence of unhandled breaking changes in the configuration. Vite and TypeScript configurations are also properly set up for the upgraded versions.
TypeScript is a build/dev-time tool and should not be in runtime dependencies
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/package.json (1)
1-1: Fix typo in PR title.The PR title contains a typo: "packadges" should be "packages".
🧹 Nitpick comments (1)
src/ui/package.json (1)
54-74: Consider consistent versioning strategy.The devDependencies use a mix of exact versions (lines 55, 56, 58, 63, 69) and caret ranges (lines 59, 60, 62, 64, 65, 66, 67, 70, 71, 72, 73). While this might be intentional for strict control of specific tooling, consider documenting the rationale or standardizing to caret ranges for flexibility in patch updates.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/ui/package.json
⏰ 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). (8)
- GitHub Check: tests (chromium)
- GitHub Check: tests (webkit)
- GitHub Check: tests (firefox)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
🔇 Additional comments (3)
src/ui/package.json (3)
9-10: LGTM: Lint scripts simplified appropriately.The lint scripts have been appropriately simplified to work with the new ESLint flat config. Using
--fixfor local development and omitting it for CI is the correct pattern.
70-70: LGTM: TypeScript correctly placed in devDependencies.TypeScript is correctly placed in devDependencies, which resolves the concern raised in the previous review. This is the appropriate location for build-time tooling.
19-52: All package versions in src/ui/package.json are valid, exist on npm, and resolve without conflicts. No security vulnerabilities or compatibility issues detected. The dependencies are recent (vue@3.5.26 published Dec 2025, apache-arrow@21.1.0 published Oct 2025) and successfully resolve to 795 total packages without conflicts.
- Add missing runId and exit properties to BlueprintExecutionLog type - Add ts-expect-error for Monaco TypeScript API compatibility issue
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/ui/src/builder/builderManager.ts (2)
36-37: Consider adding JSDoc comments for the new fields.The new
runIdandexitfields enable tracking blueprint execution runs, but their purpose and usage aren't immediately clear without documentation. Adding brief JSDoc comments would improve maintainability.💡 Example documentation
}[]; + /** Unique identifier for this blueprint execution run */ runId?: string; + /** Indicates whether this blueprint execution has exited */ exit?: boolean; };
417-432: Consider usingsome()for clearer intent and better performance.The current implementation uses
filter()[0]to check if any active log entries exist. Usingsome()would more clearly express the intent and avoid creating an intermediate array.🔎 Suggested refactor
const activeBlueprintRunId = computed(() => { const logEntries = getLogEntries(); const runId = logEntries.find((entry) => { return !!entry.blueprintExecution; })?.blueprintExecution?.runId; if (!runId) return null; - const isActive = Boolean( - logEntries.filter((entry) => { - return ( - entry?.blueprintExecution?.runId === runId && - !entry?.blueprintExecution?.exit - ); - })?.[0], - ); + const isActive = logEntries.some((entry) => + entry?.blueprintExecution?.runId === runId && + !entry?.blueprintExecution?.exit + ); return isActive ? runId : null; });Note: The
?.[0]optional chaining is unnecessary sincefilter()always returns an array (nevernullorundefined).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/ui/src/builder/builderEditorWorker.tssrc/ui/src/builder/builderManager.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ui/src/builder/builderEditorWorker.ts
⏰ 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). (8)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: tests (webkit)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
| const MAX_DISTANCE_FROM_CANDIDATE_PX = 30; | ||
| const dragDropMimeRegex = | ||
| /^application\/json;writer=(?<componentType>\w+),(?<componentId>[\w\-]*)$/; | ||
| /^application\/json;writer=(?<componentType>\w+),(?<componentId>[\w-]*)$/; |
There was a problem hiding this comment.
Is this intentional or a linter fix?
There was a problem hiding this comment.
This is a linter fix.
| height: 100%; | ||
| } | ||
|
|
||
| .mainGrid--fullWidth { |
There was a problem hiding this comment.
I think we use this one, will this be covered by this builderMainWrapper--fullWidth ?
There was a problem hiding this comment.
Good catch! The code actually uses mainGrid--fullWidth on the ShareResizeVertical component, not builderMainWrapper--fullWidth.
| border-radius: 0; | ||
| border-left: none; | ||
| border-top: none; | ||
| border-bottom: none; |
There was a problem hiding this comment.
after upgrade I found that we need changes there and avoid UI issues so I've added some fixes and implements.
|
|
||
| .mainGrid--fullWidth { | ||
| grid-column: 1 / -1; | ||
| .builderMainWrapper { |
There was a problem hiding this comment.
Where do we use this new class?
There was a problem hiding this comment.
The .builderMainWrapper class is not currently used in the template.
| .sidebar { | ||
| grid-column: 1 / 2; | ||
| grid-row: 2 / 5; | ||
| grid-row: 2 / 3; |
There was a problem hiding this comment.
Previously, is this one set incorrectly?
There was a problem hiding this comment.
Yes, grid-row: 2 / 5 was incorrect. The grid has 3 rows, and the sidebar should only span row 2 (the main content area), not rows 2-4. The fix to grid-row: 2 / 3 is correct and ensures the sidebar is properly constrained to the main content area.
There was a problem hiding this comment.
@as-flow is this something needed for shared blueprints?
There was a problem hiding this comment.
I guess that break preview mode & Vault UI
| margin-top: 4px; | ||
| } | ||
|
|
||
| .pickerContainer { |
There was a problem hiding this comment.
Where do we use this new class?
There was a problem hiding this comment.
it's an UI style improvement
after:
before:
Remove unused .builderMainWrapper and .builderMainWrapper--fullWidth classes that were defined but never used in the template. The layout uses mainGrid--fullWidth instead. Addresses review comments from bybash on PR #1256
Restore .builderMainWrapper and .builderMainWrapper--fullWidth classes as they were intended for future use. The classes are currently unused but kept for planned implementation.
madeindjs
left a comment
madeindjs
left a comment
There was a problem hiding this comment.
The scrollbar switched to black, but it's grey on dev
| .sidebar { | ||
| grid-column: 1 / 2; | ||
| grid-row: 2 / 5; | ||
| grid-row: 2 / 3; |
There was a problem hiding this comment.
I guess that break preview mode & Vault UI
There was a problem hiding this comment.
The color scheme changes make the scrollbar black
Summary by CodeRabbit
Style
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.