Skip to content

Fix dialogs markup #1709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lyubov-voloshko merged 3 commits into from
Apr 8, 2026
Merged

Fix dialogs markup #1709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ff72ec
add rule to claude.md regarding forms in mat-dialog
lyubov-voloshko Apr 8, 2026
d22d81f
wrap dialogs in form
lyubov-voloshko Apr 8, 2026
7653bcd
Merge branch 'main' into fix-dialogs-markup
lyubov-voloshko Apr 8, 2026
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
15 changes: 15 additions & 0 deletions frontend/CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,21 @@ Database configurations are defined in `src/app/consts/databases.ts`.
- Example: `@if (condition) { ... }` instead of `<div *ngIf="condition">...</div>`
- Example: `@for (item of items; track item.id) { ... }` instead of `<div *ngFor="let item of items">...</div>`

### Mat-Dialog Forms
- **When a `mat-dialog` contains form elements and presumes form submission, `mat-dialog-content` and `mat-dialog-actions` MUST be wrapped in a `<form>` tag with an `(ngSubmit)` handler, and the primary action button inside `mat-dialog-actions` must use `type="submit"`.** This ensures the dialog properly supports keyboard submission (Enter key) and follows accessibility best practices.
- Example:
```html
<form [formGroup]="myForm" (ngSubmit)="handleSubmit()">
<mat-dialog-content>
<!-- form fields -->
</mat-dialog-content>
<mat-dialog-actions>
<button mat-button type="button" mat-dialog-close>Cancel</button>
<button mat-flat-button type="submit" color="primary">Submit</button>
</mat-dialog-actions>
</form>
```

### Naming Conventions
- **Files**: `kebab-case.component.ts`
- **Classes**: `PascalCase` (e.g., `DbTableSettingsComponent`)
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/app/components/company/invite-member-dialog/invite-member-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ <h1 mat-dialog-title>Add member to <strong>{{company.name}}</strong> company</h1
</app-turnstile>
<mat-dialog-actions align="end">
<button type="button" mat-flat-button mat-dialog-close>Cancel</button>
<button mat-flat-button color="primary"
<button type="submit" mat-flat-button color="primary"
[disabled]="submitting || addUserForm.form.invalid || (isSaas && !turnstileToken)">
Add
</button>
Expand Down
59 changes: 29 additions & 30 deletions ...ponents/connect-db/db-connection-delete-dialog/db-connection-delete-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,40 @@
<h1 mat-dialog-title>Delete <strong>{{ connectionName }}</strong> connection</h1>
<form #deleteConnectionForm="ngForm" class="form" (ngSubmit)="deleteConnection()">
<mat-dialog-content>
<p class="mat-body">
Before you delete the connection, please let us know why.
Every bit of feedback helps!
</p>
<br>
<form action="" #deleteConnectionForm="ngForm" class="form">
<mat-radio-group required
name="reasons"
class="radio-group"
[(ngModel)]="reason">
<mat-radio-button value='missing-features' class="radio-button">
Missing features I need
</mat-radio-button>
<mat-radio-button value='technical-issues' class="radio-button">
Technical issues
</mat-radio-button>
<mat-radio-button value='another-product' class="radio-button">
Another product
</mat-radio-button>
<mat-radio-button value='other' class="radio-button">
Other
</mat-radio-button>
</mat-radio-group>
<mat-form-field appearance="outline" class="message">
<mat-label>Anything you want to share? (optional)</mat-label>
<textarea matInput resizeToFitContent rows="8" name="message"
[(ngModel)]="message"
></textarea>
</mat-form-field>
</form>
<mat-radio-group required
name="reasons"
class="radio-group"
[(ngModel)]="reason">
<mat-radio-button value='missing-features' class="radio-button">
Missing features I need
</mat-radio-button>
<mat-radio-button value='technical-issues' class="radio-button">
Technical issues
</mat-radio-button>
<mat-radio-button value='another-product' class="radio-button">
Another product
</mat-radio-button>
<mat-radio-button value='other' class="radio-button">
Other
</mat-radio-button>
</mat-radio-group>
<mat-form-field appearance="outline" class="message">
<mat-label>Anything you want to share? (optional)</mat-label>
<textarea matInput resizeToFitContent rows="8" name="message"
[(ngModel)]="message"
></textarea>
</mat-form-field>
</mat-dialog-content>
<mat-dialog-actions align="end">
<button mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>
<button mat-flat-button color="warn"
[disabled]="submitting"
(click)="deleteConnection()">
<button type="button" mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

mat-dialog-close is duplicated on this button (mat-dialog-close mat-dialog-close="cancel"). Please remove the redundant attribute and keep a single close value/binding to avoid invalid markup.

Suggested change
<button type="button" mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>
<button type="button" mat-flat-button mat-dialog-close="cancel">Abort</button>

Copilot uses AI. Check for mistakes.

@coderabbitai coderabbitai Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Duplicate mat-dialog-close attributes.

The button has both mat-dialog-close (without value) and mat-dialog-close="cancel". Only the second one is needed.

Proposed fix 
-  <button type="button" mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>
+  <button type="button" mat-flat-button mat-dialog-close="cancel">Abort</button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button type="button" mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>
<button type="button" mat-flat-button mat-dialog-close="cancel">Abort</button>
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/connect-db/db-connection-delete-dialog/db-connection-delete-dialog.component.html`
at line 34, In the DbConnectionDeleteDialogComponent template, remove the
duplicate mat-dialog-close attribute on the Abort button so only the
value-bearing attribute remains; specifically, delete the bare mat-dialog-close
and keep mat-dialog-close="cancel" on the <button> element to avoid duplicate
attributes.

<button type="submit" mat-flat-button color="warn"
[disabled]="submitting">
Proceed
</button>
</mat-dialog-actions>
</mat-dialog-actions>
</form>
6 changes: 4 additions & 2 deletions ...nnections-list/hosted-database-rename-dialog/hosted-database-rename-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
<h1 mat-dialog-title>Name your connection</h1>

<form (ngSubmit)="save()">
<mat-dialog-content class="rename-dialog__content">
<p class="rename-dialog__description">Give your hosted database a friendly title so you can find it easily.</p>
<mat-form-field appearance="outline" class="rename-dialog__field">
<mat-label>Connection title</mat-label>
<input matInput [(ngModel)]="title" placeholder="e.g. Production DB, Staging, My App" (keyup.enter)="save()" cdkFocusInitial>
<input matInput [(ngModel)]="title" name="title" placeholder="e.g. Production DB, Staging, My App" cdkFocusInitial>
</mat-form-field>
</mat-dialog-content>

<mat-dialog-actions align="end" class="rename-dialog__actions">
<button type="button" mat-button mat-dialog-close>Skip</button>
<button type="button" mat-flat-button color="accent" [disabled]="!title.trim() || saving" (click)="save()">
<button type="submit" mat-flat-button color="accent" [disabled]="!title.trim() || saving">
{{ saving ? 'Saving...' : 'Save' }}
</button>
</mat-dialog-actions>
</form>
4 changes: 2 additions & 2 deletions ...ents/dashboard/db-table-view/db-table-export-dialog/db-table-export-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ <h1 mat-dialog-title>Export</h1>
</mat-radio-group>
</mat-dialog-content>
<mat-dialog-actions align="end">
<button mat-flat-button mat-dialog-close>Cancel</button>
<button mat-flat-button type="" color="primary" mat-dialog-close="export">
<button type="button" mat-flat-button mat-dialog-close>Cancel</button>
<button type="submit" mat-flat-button color="primary" [disabled]="submitting">
Export
</button>
</mat-dialog-actions>
Expand Down
11 changes: 7 additions & 4 deletions ...nents/dashboard/db-tables-list/db-folder-edit-dialog/db-folder-edit-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ <h1 mat-dialog-title>
Edit folder
</h1>

<form (ngSubmit)="onSave()">
<mat-dialog-content class="dialog-body">
<!-- Folder Name Section -->
<div class="folder-name-section">
<mat-form-field appearance="outline" class="folder-name-field">
<mat-label>Folder name</mat-label>
<input matInput
type="text"
name="folderName"
[(ngModel)]="folder.name"
[disabled]="isAllTablesFolder()"
required
Expand Down Expand Up @@ -49,7 +51,7 @@ <h4>Manage tables in this folder</h4>
<div class="table-list">
<div *ngFor="let table of tables" class="table-item">
<span class="table-name">{{getTableName(table)}}</span>
<button mat-button
<button mat-button type="button"
class="action-button"
[class.add-button]="!isTableInFolder(table)"
[class.remove-button]="isTableInFolder(table)"
Expand All @@ -62,6 +64,7 @@ <h4>Manage tables in this folder</h4>
</mat-dialog-content>

<mat-dialog-actions align="end">
<button mat-button (click)="onCancel()">Cancel</button>
<button mat-button color="primary" (click)="onSave()">Save</button>
</mat-dialog-actions>
<button type="button" mat-button (click)="onCancel()">Cancel</button>
<button type="submit" mat-button color="primary">Save</button>
</mat-dialog-actions>
</form>
10 changes: 5 additions & 5 deletions ...sted-databases/hosted-database-rename-dialog/hosted-database-rename-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
<h1 mat-dialog-title>Rename <strong>{{ data.databaseName }}</strong></h1>
<form (ngSubmit)="save()">
<mat-dialog-content>
<mat-form-field appearance="outline" class="rename-dialog__field">
<mat-label>Connection name</mat-label>
<input matInput [(ngModel)]="title" placeholder="e.g. Production DB, Staging, My App"
(keyup.enter)="save()" cdkFocusInitial>
<input matInput [(ngModel)]="title" name="title" placeholder="e.g. Production DB, Staging, My App" cdkFocusInitial>
</mat-form-field>
</mat-dialog-content>
<mat-dialog-actions align="end">
<button type="button" mat-flat-button mat-dialog-close>Cancel</button>
<button type="button" mat-flat-button color="accent"
[disabled]="!title.trim() || submitting()"
(click)="save()">
<button type="submit" mat-flat-button color="accent"
[disabled]="!title.trim() || submitting()">
{{ submitting() ? 'Saving...' : 'Save' }}
</button>
</mat-dialog-actions>
</form>
9 changes: 5 additions & 4 deletions ...d/src/app/components/secrets/master-password-dialog/master-password-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ <h2 mat-dialog-title>
Master Password Required
</h2>

<form (ngSubmit)="onSubmit()">
<mat-dialog-content>
<p class="description">
This secret is protected with master password encryption.
Expand All @@ -12,9 +13,9 @@ <h2 mat-dialog-title>
<mat-form-field appearance="outline" class="full-width">
<mat-label>Master Password</mat-label>
<input matInput
name="masterPassword"
[(ngModel)]="masterPassword"
[type]="showPassword ? 'text' : 'password'"
(keyup.enter)="onSubmit()"
placeholder="Enter master password"
data-testid="master-password-input">
<button mat-icon-button matSuffix type="button"
Expand All @@ -27,11 +28,11 @@ <h2 mat-dialog-title>
</mat-dialog-content>

<mat-dialog-actions align="end">
<button mat-button mat-dialog-close>Cancel</button>
<button mat-flat-button color="primary"
(click)="onSubmit()"
<button type="button" mat-button mat-dialog-close>Cancel</button>
<button type="submit" mat-flat-button color="primary"
data-testid="master-password-submit-button">
<mat-icon>lock_open</mat-icon>
Unlock
</button>
</mat-dialog-actions>
</form>
49 changes: 24 additions & 25 deletions ...c/app/components/user-settings/account-delete-dialog/account-delete-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,35 @@
<h1 mat-dialog-title>Delete <strong>{{ userEmail }}</strong> account</h1>
<form #deleteConnectionForm="ngForm" class="form" (ngSubmit)="openDeleteConfirmation()">
<mat-dialog-content>
<p class="mat-body">
Tell us why are you deleting your account.
</p>
<br>
<form action="" #deleteConnectionForm="ngForm" class="form">
<mat-radio-group required
name="reasons"
class="radio-group"
[(ngModel)]="reason">
<div *ngFor="let reasonItem of reasonsList">
<mat-radio-button [value]="reasonItem.id" class="radio-button">
{{ reasonItem.caption }}
</mat-radio-button>
<p *ngIf="reasonItem.message && reason === reasonItem.id"
[innerHtml]="reasonItem.message"
class="mat-body-1 radio-group__message">
</p>
</div>
</mat-radio-group>
<mat-form-field appearance="outline" class="message">
<mat-label>Anything you want to share? (optional)</mat-label>
<textarea matInput resizeToFitContent rows="3" name="message"
[(ngModel)]="message"
></textarea>
</mat-form-field>
</form>
<mat-radio-group required
name="reasons"
class="radio-group"
[(ngModel)]="reason">
<div *ngFor="let reasonItem of reasonsList">
<mat-radio-button [value]="reasonItem.id" class="radio-button">
{{ reasonItem.caption }}
</mat-radio-button>
<p *ngIf="reasonItem.message && reason === reasonItem.id"
[innerHtml]="reasonItem.message"
class="mat-body-1 radio-group__message">
</p>
</div>
</mat-radio-group>
<mat-form-field appearance="outline" class="message">
<mat-label>Anything you want to share? (optional)</mat-label>
<textarea matInput resizeToFitContent rows="3" name="message"
[(ngModel)]="message"
></textarea>
</mat-form-field>
</mat-dialog-content>
<mat-dialog-actions align="end">
<button mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>
<button mat-flat-button color="warn"
(click)="openDeleteConfirmation()">
<button type="button" mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

mat-dialog-close is specified twice on this button (mat-dialog-close mat-dialog-close="cancel"). Duplicate attributes are invalid HTML and can lead to confusing behavior; keep a single close binding (e.g., only mat-dialog-close="cancel" or a single [mat-dialog-close] binding).

Suggested change
<button type="button" mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>
<button type="button" mat-flat-button mat-dialog-close="cancel">Abort</button>

Copilot uses AI. Check for mistakes.

@coderabbitai coderabbitai Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Duplicate mat-dialog-close attribute.

The Abort button has mat-dialog-close specified twice. The second instance with the value will override the first, but this is likely unintentional and should be cleaned up.

🔧 Proposed fix 
-  <button type="button" mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>
+  <button type="button" mat-flat-button mat-dialog-close="cancel">Abort</button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button type="button" mat-flat-button mat-dialog-close mat-dialog-close="cancel">Abort</button>
<button type="button" mat-flat-button mat-dialog-close="cancel">Abort</button>
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/components/user-settings/account-delete-dialog/account-delete-dialog.component.html`
at line 30, The Abort button element in account-delete-dialog.component.html
incorrectly includes the mat-dialog-close attribute twice; remove the duplicate
plain mat-dialog-close attribute and keep the value form
mat-dialog-close="cancel" on the <button> so the dialog close payload is
explicit (locate the Abort button element in
account-delete-dialog.component.html and delete the redundant mat-dialog-close
occurrence).

<button type="submit" mat-flat-button color="warn">
Delete
</button>
</mat-dialog-actions>
</form>
Loading