GoogleChrome / GoogleChrome/webstatus.dev

[ENHANCEMENT] Refactor Frontend Modal Architecture for Notification Channels

Open
#2,342 0 comments 0 reactions 1 assignee Claimed by @neilv-g View on GitHub
enhancement
Dominant language
Go
Stars
254
Forks
62
Avg merge
1d 10h
Merged PRs (30d)
64

Description

## Problem
Currently, the frontend manages state using independent boolean flags and loose ID properties. This makes it difficult to track valid UI states and forces unsafe `as` casting.

## Proposed Strategy:

### 1. Unified Modal State (State Machine)
Bind the **Mode** and **Data** into a single discriminated union property.

```typescript
type UIState =
| { mode: 'idle' }
| { mode: 'delete'; channel: NotificationChannelResponse }
| { mode: 'edit'; channel: NotificationChannelResponse }
| { mode: 'create' };

@state() private _uiState: UIState = { mode: 'idle' };

// Transitioning is atomic and type-safe!
private _handleDelete(channel: NotificationChannelResponse) {
this._uiState = { mode: 'delete', channel };
}
```

### 2. Registry Inversion (Direct Injection)
Move type-narrowing to the Registry. This allows forms to be "Dumb UI" components that simply receive the data they need.

```typescript
// channel-config-registry.ts
case 'webhook': {
// Narrowing happens ONCE here
const config = channel?.config?.type === 'webhook' ? channel.config : undefined;
return html`
`;
}
```

### 3. Clean Switch-based Rendering
Use a helper method to map state to templates. TypeScript automatically narrows `this._uiState` in each `case`.

```typescript
render() {
return html`${this.renderModals()}`;
}

private renderModals() {
switch (this._uiState.mode) {
case 'delete':
// Narrowed: this._uiState.channel is guaranteed to exist
return html`

Are you sure? This cannot be undone.
Delete
`;
case 'edit':
return html``;
default:
return nothing;
}
}
```

## Expected Outcomes
- **Zero Casting**: Eliminates `as unknown as WebhookConfig`.
- **Illegal State Prevention**: You can't be in 'delete' mode without a channel.
- **Improved Testability**: Deterministic mapping from `_uiState` value to DOM state.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.