Make maximum window limit configurable (#2880) #2921
Conversation
Reviewer's GuideThis PR replaces hardcoded maximum window limits with a user-configurable Sequence diagram for passing maxWindows setting from store to WindowSwitcherComponentsequenceDiagram
participant Store as "Settings Store"
participant Header as "HeaderComponent"
participant Switcher as "WindowSwitcherComponent"
Store->>Header: Provide settings (including maxWindows)
Header->>Switcher: Pass [maxWindowCount]=settings()?.maxWindows
Switcher->>Switcher: Use maxWindowCount input for window limit logic
Class diagram for updated SettingsState interfaceclassDiagram
class SettingsState {
+theme: string
+language: string
+addQueryDepthLimit: number
+tabSize: number
+maxWindows: number
+"introspection.options.inputValueDeprecation": boolean
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe PR introduces a configurable maximum windows setting by adding an optional Changes
Sequence DiagramsequenceDiagram
participant Store as Settings Store
participant Header as Header Component
participant Switcher as Window Switcher
participant Template as Template
Store->>Store: Initial state includes maxWindows<br/>from altairConfig.max_windows
Note over Store: SettingsState { maxWindows?: number }
rect rgb(230, 245, 230)
Header->>Header: settings() reactive signal
Header->>Switcher: [maxWindowCount]="settings()?.maxWindows"
end
rect rgb(245, 235, 230)
Switcher->>Switcher: input(maxWindowCount)
Switcher->>Template: Render conditional<br/>based on maxWindowCount()
end
Template->>Template: Show ADD_NEW_WINDOW button<br/>if windows < maxWindowCount()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span multiple architectural layers (types, store, components, i18n) and introduce a reactive input refactor to the Window Switcher component. While the logic is straightforward and follows existing patterns, the distributed nature of changes across type definitions, state management, and component lifecycle requires careful verification of data flow consistency. Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @ishikabhoyar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a long-standing limitation by making the maximum number of open windows in Altair user-configurable. This change provides greater flexibility for users to tailor the application to their specific workflow and system capabilities, whether they need more windows for complex tasks or fewer for resource optimization. The implementation ensures backward compatibility and maintains sensible default values while allowing customization via settings. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider validating and clamping user‐provided maxWindows to ensure it stays within a sensible range (e.g., ≥1 and below a performance threshold).
- Add the new SETTINGS_MAX_WINDOWS_TEXT key to all i18n files so the settings UI can display the label correctly.
- Add a component or integration test to verify that the window-switcher hides the “Add New Window” action when the maxWindows limit is reached.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider validating and clamping user‐provided maxWindows to ensure it stays within a sensible range (e.g., ≥1 and below a performance threshold).
- Add the new SETTINGS_MAX_WINDOWS_TEXT key to all i18n files so the settings UI can display the label correctly.
- Add a component or integration test to verify that the window-switcher hides the “Add New Window” action when the maxWindows limit is reached.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively introduces a configurable maxWindows limit, moving away from hardcoded values. The changes are well-implemented across the application, including state management, components, and tests. The addition of the CONFIGURABLE_MAX_WINDOWS.md documentation file is particularly helpful.
My review includes a critical suggestion to add input validation for the new maxWindows setting to prevent potential issues from invalid user input. I've also included a couple of minor suggestions to clean up the new documentation file. Overall, this is a solid contribution.
| readonly activeWindowId = input(''); | ||
| readonly isElectron = input(false); | ||
| readonly enableScrollbar = input(false); | ||
| readonly maxWindowCount = input(this.altairConfig.max_windows); |
There was a problem hiding this comment.
The maxWindows setting comes from user-configured JSON and could be a non-numeric value, a float, or a negative number. This could lead to unexpected behavior in the template where it's used for comparison (windowIds().length < maxWindowCount()), as string comparison is lexicographical (e.g., '10' < '2'). It's safer to validate and coerce the input value. You can use the transform option on the input signal to ensure maxWindowCount is always a valid integer.
readonly maxWindowCount = input(this.altairConfig.max_windows, {
transform: (value: unknown) => {
if (value === undefined || value === null) {
return this.altairConfig.max_windows;
}
const num = Number(value);
// Fallback to default if not a valid non-negative number
if (isNaN(num) || num < 0) {
return this.altairConfig.max_windows;
}
return Math.floor(num);
},
});| language: <SettingsLanguage>altairConfig.default_language, | ||
| addQueryDepthLimit: altairConfig.add_query_depth_limit, | ||
| tabSize: altairConfig.tab_size, | ||
| maxWindows: altairConfig.max_windows, // NEW |
There was a problem hiding this comment.
This documentation file is a great addition! For better clarity in the final documentation, it would be best to remove the // NEW comment from this code example, as it appears to be a note for the pull request review process.
| maxWindows: altairConfig.max_windows, // NEW | |
| maxWindows: altairConfig.max_windows, |
| [isElectron]="isElectron()" | ||
| [collections]="collections()" | ||
| [enableScrollbar]="settings()?.enableTablistScrollbar" | ||
| [maxWindowCount]="settings()?.maxWindows" <!-- NEW --> |
There was a problem hiding this comment.
To keep the code examples in the documentation clean, consider removing the <!-- NEW --> comment. It seems like a temporary note for the review.
| [maxWindowCount]="settings()?.maxWindows" <!-- NEW --> | |
| [maxWindowCount]="settings()?.maxWindows" |
![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)
packages/altair-core/src/types/state/settings.interfaces.ts (1)
196-200: Consider adding validation constraints.While the type definition is correct, there's no validation to ensure
maxWindowsreceives reasonable values. Users could potentially set it to 0, negative numbers, or excessively high values that could impact performance.Consider one of these approaches:
- Add JSDoc constraints (e.g.,
@minimum 1,@maximum 500) for documentation purposes- Add runtime validation in the settings reducer to clamp values to a safe range
- Add a custom validator if this will be exposed in a UI form
Example JSDoc addition:
/** * Maximum number of windows/tabs allowed + * @minimum 1 + * @maximum 500 * @default 50 (Electron), 15 (Web) */ maxWindows?: number;CONFIGURABLE_MAX_WINDOWS.md (1)
1-201: Well-structured documentation with comprehensive coverage.This documentation file provides excellent coverage of the feature including:
- Clear overview and rationale
- Detailed implementation changes with code examples
- Usage instructions for different audiences
- Default values and backward compatibility notes
However, consider the following improvements:
Validation section (lines 154-165): The documentation correctly identifies that validation is missing and suggests it as a future enhancement. This aligns with the earlier review comment on the type definition file. Consider opening an issue to track this improvement and linking it in the documentation.
"Next Steps" section (lines 183-192): Some items like "Run
pnpm bootstrap" are one-time setup tasks that will become obsolete once merged. Consider rephrasing this as a "Testing Checklist" or removing time-sensitive items after the PR is merged.Example refinement for the validation section:
## Validation The feature currently doesn't include explicit validation for the `maxWindows` value. Users should: - Use reasonable positive integers (e.g., 1-200) - Consider their system's memory and performance capabilities - Avoid setting the value too low (minimum 1 is recommended) -Future enhancements could include: +**TODO** (track in issue #XXXX): Future enhancements should include: - Min/max validation in the settings UI - Performance warnings for very high values - Automatic adjustment based on system resources
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CONFIGURABLE_MAX_WINDOWS.md(1 hunks)packages/altair-app/src/app/modules/altair/components/header/header.component.html(1 hunks)packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.html(1 hunks)packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.ts(1 hunks)packages/altair-app/src/app/modules/altair/store/settings/settings.reducer.ts(1 hunks)packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts(8 hunks)packages/altair-app/src/assets/i18n/en-US.json(1 hunks)packages/altair-core/src/types/state/settings.interfaces.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.ts
📄 CodeRabbit inference engine (.github/instructions/app-testing.instructions.md)
Follow project code style using ESLint and Prettier
**/*.ts: Use explicit type annotations for function parameters and return types
Prefer interfaces over type aliases for object shapes
Use union types and literal types for better type safety
Leverage generic types for reusable components
Group imports: external libraries first, then internal modules
Use absolute imports from package roots when possible
Prefer named exports over default exports
Use custom error classes that extend Error
Implement proper error boundaries and handling
Log errors with sufficient context for debugging
Use observables (RxJS) for reactive programming patterns where appropriate
Manage subscriptions to avoid memory leaks
Use appropriate RxJS operators for data transformation
Handle errors in observable streams
Use async/await for sequential operations
Handle promise rejections properly
Use Promise.all() for concurrent operations
Implement timeout handling for long-running operations
Dispose of resources properly (subscriptions, event listeners)
Use weak references where appropriate
Avoid creating unnecessary objects in hot paths
Profile memory usage for performance-critical code
Use tree-shaking-friendly imports
Lazy load heavy modules when possible
Monitor bundle size impacts of new dependencies
Use dynamic imports for code splitting
Validate and sanitize all user inputs
Implement proper XSS and injection prevention
Validate API responses before processing
Sanitize sensitive data in logs
Follow secure coding practices
Group related functionality in modules
Keep files focused and not too large
Use consistent naming conventions
Organize imports and exports clearly
Write JSDoc comments for public APIs
Keep documentation up to date with code changes (inline docs)
Use meaningful variable and function names
Handle environment-specific APIs properly
Use TypeScript features appropriate for the configured version
Files:
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.tspackages/altair-app/src/app/modules/altair/store/settings/settings.reducer.tspackages/altair-app/src/app/modules/altair/store/settings/settings.spec.tspackages/altair-core/src/types/state/settings.interfaces.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Use TypeScript for implementation across the codebase
Files:
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.tspackages/altair-app/src/app/modules/altair/store/settings/settings.reducer.tspackages/altair-app/src/app/modules/altair/store/settings/settings.spec.tspackages/altair-core/src/types/state/settings.interfaces.ts
packages/altair-app/**/*.{ts,html}
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Implement and modify the main web app using Angular conventions within packages/altair-app
Files:
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.tspackages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.htmlpackages/altair-app/src/app/modules/altair/components/header/header.component.htmlpackages/altair-app/src/app/modules/altair/store/settings/settings.reducer.tspackages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
packages/altair-app/src/app/modules/altair/components/**
📄 CodeRabbit inference engine (.github/instructions/angular-components.instructions.md)
Place all Angular components under packages/altair-app/src/app/modules/altair/components/
Files:
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.tspackages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.htmlpackages/altair-app/src/app/modules/altair/components/header/header.component.html
packages/altair-app/src/app/modules/altair/components/**/*.component.ts
📄 CodeRabbit inference engine (.github/instructions/angular-components.instructions.md)
packages/altair-app/src/app/modules/altair/components/**/*.component.ts: Use kebab-case for component file names (e.g., query-editor.component.ts)
Use ChangeDetectionStrategy.OnPush for components when possible
Implement appropriate Angular lifecycle hooks (e.g., OnInit, OnDestroy)
Keep each component single-responsibility with one clear purpose
Use Angular dependency injection for services and dependencies
Use reactive patterns with RxJS observables for state
Manage subscriptions and unsubscribe/cleanup in ngOnDestroy
Emit events with Angular EventEmitter for parent-child communication
Expose component API via @input and @output
Avoid memory leaks by properly unsubscribing from observables
Files:
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.ts
packages/altair-app/src/app/modules/altair/components/**/*.{component.ts,component.html,component.scss,component.spec.ts}
📄 CodeRabbit inference engine (.github/instructions/angular-components.instructions.md)
Each component should have its own directory containing its .component.ts, .component.html, .component.scss, and .component.spec.ts files
Files:
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.tspackages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.htmlpackages/altair-app/src/app/modules/altair/components/header/header.component.html
packages/altair-app/src/app/modules/altair/components/**/*.component.{ts,html}
📄 CodeRabbit inference engine (.github/instructions/angular-components.instructions.md)
packages/altair-app/src/app/modules/altair/components/**/*.component.{ts,html}: Use ng-zorro Ant Design components for UI
Follow existing patterns for modals, forms, and navigation
Use Angular reactive forms for complex forms
Implement proper form validation and error handling
Files:
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.tspackages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.htmlpackages/altair-app/src/app/modules/altair/components/header/header.component.html
packages/altair-app/src/app/modules/altair/components/**/*.component.html
📄 CodeRabbit inference engine (.github/instructions/angular-components.instructions.md)
packages/altair-app/src/app/modules/altair/components/**/*.component.html: Provide trackBy functions for *ngFor to improve performance
Include appropriate ARIA attributes for accessibility
Ensure keyboard navigation works (e.g., focus management, key handlers)
Use semantic HTML elements in templates
Files:
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.htmlpackages/altair-app/src/app/modules/altair/components/header/header.component.html
{**/__tests__/**/*.ts,**/*.{spec,test}.ts}
📄 CodeRabbit inference engine (.github/instructions/app-testing.instructions.md)
{**/__tests__/**/*.ts,**/*.{spec,test}.ts}: Use Jest as the testing framework for all tests
Organize tests next to the code under test: use a tests folder or .test.ts/.spec.ts files alongside sources
Use clear, descriptive test names explaining what is being verified
Mock dependencies with Jest to isolate the unit under test
Leverage TypeScript types in tests; define interfaces/types for expected data shapes
Files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
**/*.{spec,test}.{ts,tsx,js}
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Write and maintain tests; Jest is used for most testing
Files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
**/*.{test,spec}.{ts,js}
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
**/*.{test,spec}.{ts,js}: Follow the Arrange-Act-Assert (AAA) pattern in tests
Write descriptive test names that explain expected behavior
Keep tests focused and independent
Use consistent naming conventions across all test files
Group related tests using describe blocks
Use nested describe blocks for different methods or scenarios
Place setup code in beforeEach or beforeAll hooks
Clean up resources in afterEach or afterAll hooks
Mock external dependencies to isolate units under test
Use Jest's mocking capabilities effectively
Create reusable mock factories for common dependencies
Verify interactions with mocked dependencies when necessary
Use async/await for testing promises
Test both success and error scenarios in async code
Handle timeouts appropriately in async tests
Test concurrent operations when relevant
For NestJS controllers, test HTTP handling, response formatting, auth, and error/status codes; mock service dependencies
For NestJS services, test business logic, data transformations, error handling/validation, and verify logging/monitoring calls
For API integration tests, test endpoints end-to-end, use test DB/transactions, test auth flows, and verify API contracts/responses
For browser extensions, mock browser APIs (chrome., browser.), test message passing, content scripts, and verify manifest configuration
Write performance tests for critical code paths and set performance budgets/thresholds
Monitor test execution times and profile memory usage in tests
Load test API endpoints, verify graceful degradation, check for resource cleanup/memory leaks, and monitor performance metrics
E2E tests should focus on critical user journeys, use realistic data, test cross-browser, and verify integrations
Use dedicated test environments, mock external services appropriately, ensure data consistency, and clean up test artifacts
Create reusable test data factories and use realistic but anonymized data; version fixtures with code and clean up after tests
Maintain high t...
Files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
packages/altair-app/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
packages/altair-app/**/*.{test,spec}.ts: Use the custom testing framework in packages/altair-app/src/testing for Angular component tests
Focus on component business logic rather than UI library behavior
Mock services and external dependencies in component tests
Test component lifecycle methods appropriately
In components, test methods, business logic, event emissions, state changes, lifecycle, and integration with injected services
Do NOT test UI library component properties, template rendering details, CSS, or third-party library behavior in component tests
Files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{spec,test}.ts: Write unit tests focusing on business logic and behavior
Use descriptive test names
Mock external dependencies appropriately in tests
Test edge cases and error conditions
Files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
packages/altair-core/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Modify shared logic/types/utilities in packages/altair-core carefully, as changes impact multiple packages; ensure thorough testing
Files:
packages/altair-core/src/types/state/settings.interfaces.ts
**/*.{md,mdx}
📄 CodeRabbit inference engine (.github/instructions/documentation.instructions.md)
**/*.{md,mdx}: Provide working code examples in documentation
Organize documentation by feature and user journey
Include getting started guides and tutorials
Provide troubleshooting and FAQ sections
Use clear, concise language
Follow consistent terminology throughout documentation
Write for the target audience (developers, end users)
Use active voice and present tense
Provide complete, working code examples
Test all code examples to ensure they work
Use realistic, relevant examples
Include both basic and advanced usage patterns
Use clear headings and subheadings
Organize content logically from basic to advanced
Use bullet points and numbered lists for clarity
Include a table of contents for long documents
Document all public functions and classes with parameter types, descriptions, return values, exceptions, and usage examples
Document all public interfaces and types with purpose, usage context, examples, and constraints
Explain features from the user's perspective
Include step-by-step instructions with screenshots in user guides
Cover common use cases and workflows
Provide troubleshooting tips in feature docs
Create migration guides for breaking changes with before/after examples and reasoning; include automated tools when possible
Document the development environment setup, prerequisites, and troubleshooting
Keep development setup instructions up to date
Document coding standards, architecture decisions, and examples of good/bad practices; reference automated tools
Release notes should highlight major features, include upgrade instructions, mention breaking changes, and link to detailed docs
Update examples to match current API
Remove outdated information from docs
Fix broken links and references
Use inclusive, accessible language; avoid jargon when possible; define technical terms; consider non-native English speakers
Use proper heading hierarchy for visual accessibility
Include alt text for images and screenshots
Ensure sufficient color contrast in embedded vi...
Files:
CONFIGURABLE_MAX_WINDOWS.md
🧠 Learnings (4)
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Test state management and property changes of components
Applied to files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
📚 Learning: 2025-10-13T05:15:52.113Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/angular-components.instructions.md:0-0
Timestamp: 2025-10-13T05:15:52.113Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.component.spec.ts : Test event emissions, state changes, and method behavior in unit tests
Applied to files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Test component interactions with services using mocks
Applied to files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
PR: altair-graphql/altair#0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.spec.ts : Mock dependencies properly in tests
Applied to files:
packages/altair-app/src/app/modules/altair/store/settings/settings.spec.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). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (6)
packages/altair-app/src/assets/i18n/en-US.json (1)
99-99: LGTM! Well-named i18n key.The new translation key follows the existing naming convention and provides a clear label for the maximum windows setting.
packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.html (1)
94-94: LGTM! Correct signal usage.The template correctly calls
maxWindowCount()as a function, which aligns with the component's change to use Angular'sinput()signal.packages/altair-app/src/app/modules/altair/components/header/header.component.html (1)
18-18: LGTM! Proper binding with optional chaining.The binding correctly passes
maxWindowsfrom settings to the window switcher component using optional chaining, which is appropriate since settings could be undefined.packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts (1)
11-11: LGTM! Comprehensive test coverage.All test cases have been properly updated to include the new
maxWindowsproperty, covering:
- Initial state construction
- User-provided initial data
- Persisted settings
- JSON string parsing with and without comments
The tests correctly verify the snake_case to camelCase conversion (
max_windows→maxWindows).Also applies to: 31-31, 51-51, 67-67, 78-78, 105-105, 121-121, 143-143
packages/altair-app/src/app/modules/altair/store/settings/settings.reducer.ts (1)
18-18: LGTM! Correct state initialization.The
maxWindowsproperty is properly initialized fromaltairConfig.max_windowsin the initial state, with appropriate override behavior from user and persisted settings via the spread operators.packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.ts (1)
31-31: LGTM! Component input with sensible fallback.The change from a class property to an input signal is correct, enabling the component to receive the
maxWindowsvalue from settings reactively. The default valuethis.altairConfig.max_windowsprovides a reasonable fallback if the parent doesn't supply the value.
There was a problem hiding this comment.
Pull Request Overview
This PR makes the maximum window limit user-configurable, replacing the hardcoded limits (50 for Electron, 15 for Web) with a new maxWindows setting. Users can now customize this limit through their settings JSON while maintaining sensible platform-specific defaults.
Key changes:
- Added
maxWindowsproperty to the settings state interface and initialized it from configuration - Converted window switcher's
maxWindowCountfrom a class property to an input signal for reactive updates - Connected settings through the component hierarchy (header → window switcher)
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/altair-core/src/types/state/settings.interfaces.ts | Added optional maxWindows property to SettingsState interface with JSDoc |
| packages/altair-app/src/app/modules/altair/store/settings/settings.reducer.ts | Initialized maxWindows from altairConfig in getInitialState |
| packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.ts | Changed maxWindowCount from class property to input signal |
| packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.html | Updated template to call maxWindowCount as a function (signal) |
| packages/altair-app/src/app/modules/altair/components/header/header.component.html | Added maxWindowCount binding to pass setting to window switcher |
| packages/altair-app/src/assets/i18n/en-US.json | Added SETTINGS_MAX_WINDOWS_TEXT translation key |
| packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts | Updated all test cases to include max_windows in mock config and expectations |
| CONFIGURABLE_MAX_WINDOWS.md | Added comprehensive documentation of the feature implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
1 participant
Fixes #2880
Description
This PR implements a configurable maximum window limit to address issue #2880.
Currently, Altair enforces hardcoded maximum window limits of 50 for Electron and 15 for web environments. This PR makes these limits user-configurable through the application's settings while maintaining sensible defaults.
Changes
maxWindowsproperty toSettingsStateinterface inpackages/altair-core/src/types/state/settings.interfaces.tsmaxWindowsfrom configmaxWindowssetting to window switcherSETTINGS_MAX_WINDOWS_TEXTfor future UI enhancementsFiles Modified
packages/altair-core/src/types/state/settings.interfaces.tspackages/altair-app/src/app/modules/altair/store/settings/settings.reducer.tspackages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.tspackages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.htmlpackages/altair-app/src/app/modules/altair/components/header/header.component.htmlpackages/altair-app/src/assets/i18n/en-US.jsonpackages/altair-app/src/app/modules/altair/store/settings/settings.spec.tsHow to Use
Users can now configure the maximum window limit by editing their settings JSON:
{ "maxWindows": 25 } ## Summary by Sourcery Make the maximum window limit user-configurable by introducing a new `maxWindows` setting with sensible Electron and Web defaults and wiring it through settings, UI components, translations, and tests. New Features: - Allow configuring the maximum number of windows via a new `maxWindows` setting instead of hardcoded limits Enhancements: - Add `maxWindows` to SettingsState and initialize it from AltairConfig defaults - Expose the configurable `maxWindows` to the window switcher and header components - Introduce a `SETTINGS_MAX_WINDOWS_TEXT` translation key for the new setting Documentation: - Add CONFIGURABLE_MAX_WINDOWS.md documentation outlining the feature, defaults, and usage Tests: - Update settings reducer unit tests to include and validate the new `maxWindows` property <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Maximum windows setting is now configurable. Users can customize the maximum number of windows allowed in the application, with platform-specific defaults (50 for Electron, 15 for Web) and full support for both persisted and programmatic configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->