Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
201 changes: 201 additions & 0 deletions CONFIGURABLE_MAX_WINDOWS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
# Configurable Maximum Windows Feature

## Overview

This feature makes the maximum window limit configurable through the application's settings, addressing the limitation of hardcoded values (50 for Electron, 15 for Web) in the codebase.

## Changes Made

### 1. Core Type Definition

**File**: `packages/altair-core/src/types/state/settings.interfaces.ts`

Added a new optional property to the `SettingsState` interface:

```typescript
/**
* Maximum number of windows/tabs allowed
* @default 50 (Electron), 15 (Web)
*/
maxWindows?: number;
```

This property allows users to override the default maximum window limits based on their workflow needs and hardware capabilities.

### 2. Settings Reducer Update

**File**: `packages/altair-app/src/app/modules/altair/store/settings/settings.reducer.ts`

Updated the `getInitialState()` function to include the default `maxWindows` value from `AltairConfig`:

```typescript
export const getInitialState = (): SettingsState => {
const altairConfig = getAltairConfig();
const initialSettings = altairConfig.initialData.settings ?? {};
return {
theme: altairConfig.defaultTheme,
language: <SettingsLanguage>altairConfig.default_language,
addQueryDepthLimit: altairConfig.add_query_depth_limit,
tabSize: altairConfig.tab_size,
maxWindows: altairConfig.max_windows, // NEW

Choose a reason for hiding this comment

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

medium

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.

Suggested change
maxWindows: altairConfig.max_windows, // NEW
maxWindows: altairConfig.max_windows,

...initialSettings,
};
};
```

This ensures that the default value from `AltairConfig` (50 for Electron, 15 for Web) is used unless overridden by the user.

### 3. Window Switcher Component

**File**: `packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.ts`

Changed `maxWindowCount` from a class property initialized with `AltairConfig` to an input signal that receives the value from settings:

```typescript
// Before:
maxWindowCount = this.altairConfig.max_windows;

// After:
readonly maxWindowCount = input(this.altairConfig.max_windows);
```

This allows the component to reactively update when the setting changes.

### 4. Header Component Template

**File**: `packages/altair-app/src/app/modules/altair/components/header/header.component.html`

Added the `maxWindowCount` binding to pass the setting value to the window switcher:

```html
<app-window-switcher
[windows]="windows()"
[windowIds]="windowIds()"
[activeWindowId]="activeWindowId()"
[closedWindows]="closedWindows()"
[isElectron]="isElectron()"
[collections]="collections()"
[enableScrollbar]="settings()?.enableTablistScrollbar"
[maxWindowCount]="settings()?.maxWindows" <!-- NEW -->

Choose a reason for hiding this comment

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

medium

To keep the code examples in the documentation clean, consider removing the <!-- NEW --> comment. It seems like a temporary note for the review.

Suggested change
[maxWindowCount]="settings()?.maxWindows" <!-- NEW -->
[maxWindowCount]="settings()?.maxWindows"

(newWindowChange)="newWindowChange.emit($event)"
...
/>
```

### 5. Internationalization

**File**: `packages/altair-app/src/assets/i18n/en-US.json`

Added a translation key for the setting label:

```json
"SETTINGS_MAX_WINDOWS_TEXT": "Maximum Windows"
```

### 6. Tests

**File**: `packages/altair-app/src/app/modules/altair/store/settings/settings.spec.ts`

Updated all test cases to include the `maxWindows` property in the mock configuration and expected results:

- Added `max_windows: 15` to the mock `AltairConfig`
- Updated all test expectations to include `maxWindows: 15` (or `maxWindows: 50` for Electron tests)

## How to Use

### For End Users

1. Open Altair GraphQL Client
2. Navigate to Settings (gear icon in the header)
3. In the settings JSON editor, add or modify the `maxWindows` property:

```json
{
"theme": "dark",
"language": "en-US",
"maxWindows": 100, // Set your desired limit
...
}
```

4. Click Save

The new limit will take effect immediately, allowing you to open more (or fewer) windows based on your preference.

### For Developers/Integrators

When embedding Altair or initializing it programmatically, you can set the initial maximum windows:

```typescript
new AltairConfig({
initialSettings: {
maxWindows: 75 // Custom limit
}
})
```

Or use persisted settings:

```typescript
new AltairConfig({
persistedSettings: {
maxWindows: 100 // This will override user settings
}
})
```

## Default Values

- **Electron**: 50 windows (unchanged)
- **Web**: 15 windows (unchanged)

These defaults remain in `packages/altair-core/src/config/index.ts` and are applied automatically if the user doesn't specify a custom value.

## 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:
- Min/max validation in the settings UI
- Performance warnings for very high values
- Automatic adjustment based on system resources

## Benefits

1. **Flexibility**: Power users can increase limits for complex workflows
2. **Performance**: Users with limited resources can decrease limits
3. **Customization**: Different environments can have different optimal limits
4. **Future-proof**: Easy to adjust as hardware capabilities improve

## Backward Compatibility

This change is fully backward compatible:

- Existing installations will use the default values
- No breaking changes to the API
- Settings files without `maxWindows` will work as before
- The feature is opt-in through settings

## Next Steps

To fully enable this feature in the UI:

1. Run `pnpm bootstrap` to build the TypeScript packages
2. Run `pnpm test` to ensure all tests pass
3. Test the feature manually in both Electron and Web environments
4. Consider adding a UI control in the settings dialog for easier configuration
5. Add validation rules to prevent unreasonable values
6. Update user documentation

## Related Files

- `packages/altair-core/src/config/index.ts` - Original hardcoded limits
- `packages/altair-core/src/types/state/settings.interfaces.ts` - Settings interface
- `packages/altair-app/src/app/modules/altair/store/settings/settings.reducer.ts` - Settings state management
- `packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.ts` - Component that uses the limit
- `packages/altair-app/src/app/modules/altair/components/window-switcher/window-switcher.component.html` - Template that checks the limit
- `packages/altair-app/src/app/modules/altair/components/header/header.component.html` - Passes setting to component
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
[isElectron]="isElectron()"
[collections]="collections()"
[enableScrollbar]="settings()?.enableTablistScrollbar"
[maxWindowCount]="settings()?.maxWindows"
(newWindowChange)="newWindowChange.emit($event)"
(activeWindowChange)="activeWindowChange.emit($event)"
(removeWindowChange)="removeWindowChange.emit($event)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
</ul>
</nz-dropdown-menu>
}
@if (windowIds().length < maxWindowCount) {
@if (windowIds().length < maxWindowCount()) {
<div class="window-switcher__item" (click)="newWindowChange.emit()">
{{ 'ADD_NEW_WINDOW_TEXT' | translate }}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export class WindowSwitcherComponent {
readonly activeWindowId = input('');
readonly isElectron = input(false);
readonly enableScrollbar = input(false);
readonly maxWindowCount = input(this.altairConfig.max_windows);

Choose a reason for hiding this comment

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

high

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);
    },
  });

readonly activeWindowChange = output<string>();
readonly newWindowChange = output();
readonly removeWindowChange = output<string>();
Expand All @@ -44,7 +45,6 @@ export class WindowSwitcherComponent {
}

windowIdEditing = '';
maxWindowCount = this.altairConfig.max_windows;

onDropEnd(event: CdkDragDrop<any, any, any>) {
this.moveWindow(event.previousIndex || 0, event.currentIndex || 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const getInitialState = (): SettingsState => {
language: <SettingsLanguage>altairConfig.default_language,
addQueryDepthLimit: altairConfig.add_query_depth_limit,
tabSize: altairConfig.tab_size,
maxWindows: altairConfig.max_windows,
...initialSettings,
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ let mockAltairConfig = {
default_language: 'en-US',
add_query_depth_limit: 1,
tab_size: 1,
max_windows: 15,
};
jest.mock('altair-graphql-core/build/config', () => {
return {
Expand All @@ -27,6 +28,7 @@ describe('settings', () => {
default_language: 'en-US',
add_query_depth_limit: 1,
tab_size: 1,
max_windows: 15,
};
});
it('should return previous state if action is not known', () => {
Expand All @@ -46,6 +48,7 @@ describe('settings', () => {
language: 'en-US',
addQueryDepthLimit: 1,
tabSize: 1,
maxWindows: 15,
});
});

Expand All @@ -61,6 +64,7 @@ describe('settings', () => {
default_language: 'en-US',
add_query_depth_limit: 1,
tab_size: 1,
max_windows: 15,
};
const newState = settingsReducer(undefined, {
type: 'UNKNOWN_ACTION',
Expand All @@ -71,6 +75,7 @@ describe('settings', () => {
language: 'en-US',
addQueryDepthLimit: 1,
tabSize: 1,
maxWindows: 15,
});
});

Expand All @@ -97,6 +102,7 @@ describe('settings', () => {
language: 'en-US',
addQueryDepthLimit: 3,
tabSize: 2,
maxWindows: 15,
});
});

Expand All @@ -112,6 +118,7 @@ describe('settings', () => {
language: 'en-US',
addQueryDepthLimit: 1,
tabSize: 1,
maxWindows: 15,
});
});

Expand All @@ -133,6 +140,7 @@ describe('settings', () => {
language: 'en-US',
addQueryDepthLimit: 1,
tabSize: 1,
maxWindows: 15,
});
});

Expand Down
1 change: 1 addition & 0 deletions packages/altair-app/src/assets/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
"SETTINGS_LANGUAGE_TEXT": "Language",
"SETTINGS_ADD_QUERY_DEPTH_LIMIT_TEXT": "Add Query Depth Limit",
"SETTINGS_TAB_SIZE_TEXT": "Tab Size",
"SETTINGS_MAX_WINDOWS_TEXT": "Maximum Windows",
"SETTINGS_HELP_WITH_TRANSLATIONS_TEXT": "Would you like to help with translations?",
"SETTINGS_SHOW_EDITOR_HINT": "Press ctrl-space to show hint",
"SETTINGS_KEYBOARD_SHORTCUTS": "Keyboard shortcuts",
Expand Down
6 changes: 6 additions & 0 deletions packages/altair-core/src/types/state/settings.interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ export interface SettingsState {
* Whether target GraphQL server supports deprecation of input values
*/
'introspection.options.inputValueDeprecation'?: boolean;

/**
* Maximum number of windows/tabs allowed
* @default 50 (Electron), 15 (Web)
*/
maxWindows?: number;
}

// Partial settings state for generating partial validator
Expand Down