Skip to content

Fixes#1436

Merged
lyubov-voloshko merged 8 commits into
mainfrom
fixes
Nov 17, 2025
Merged

Fixes#1436
lyubov-voloshko merged 8 commits into
mainfrom
fixes

Conversation

@lyubov-voloshko

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings November 17, 2025 00:18
@lyubov-voloshko lyubov-voloshko merged commit 3d40ab7 into main Nov 17, 2025
17 checks passed
@lyubov-voloshko lyubov-voloshko deleted the fixes branch November 17, 2025 00:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements multiple UI fixes and improvements across the frontend, focusing on styling consistency, mobile responsiveness, and the addition of a CSV export feature for table data.

Key Changes

  • Added CSV export functionality for selected table rows with proper data serialization handling
  • Improved mobile responsiveness for sidebar navigation, foreign key hints, and row preview panels
  • Enhanced dark theme support with proper background colors for collapsed sections and buttons
  • Restructured table list navigation with flexible layout using CSS order properties

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
foreign-key.component.html Updated hint text from "Foreign key search fields" to "Searchable foreign key columns" for consistency
foreign-key.component.css Added responsive styling for form field hints on mobile devices (≤600px)
db-tables-list.component.ts Added hasCustomFolders() helper method to determine folder layout positioning
db-tables-list.component.html Restructured expanded container with flexible ordering for search, add button, and folders sections
db-tables-list.component.css Implemented flexible layout with CSS order properties and sticky positioning for add button; improved dark theme support
db-table-view.component.ts Added exportData() method to export selected rows to CSV with nested object handling
db-table-view.component.html Added "Export to CSV" button for bulk actions when export is allowed
db-table-view.component.css Removed duplicate .hidden class definition
db-table-settings.component.html Applied consistent styling class to subheading text
db-table-settings.component.css Added theme-aware styling for subheadings and order title elements
db-table-row-view.component.css Improved mobile layout with fixed positioning for row preview sidebar
dashboard.component.css Fixed drawer container display and overflow properties; added explicit height to table preview

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

<mat-hint>Improve search performance by configuring <em>Foreign key search fields</em>&nbsp;
<a routerLink="/dashboard/{{connectionID}}/{{relations.referenced_table_name}}/settings" class="hint-link">here</a>.
<mat-hint>Improve search performance by configuring <em>Searchable foreign key columns</em>&nbsp;
<a routerLink="/dashboard/{{connectionID}}/{{relations.referenced_table_name}}/settings" class="hint-link">here</a>

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] A period (.) was removed from the end of the hint text at line 22. For consistency, hint text should either always end with punctuation or never end with it. Verify that this aligns with the project's style guide for UI text.

Suggested change
<a routerLink="/dashboard/{{connectionID}}/{{relations.referenced_table_name}}/settings" class="hint-link">here</a>
<a routerLink="/dashboard/{{connectionID}}/{{relations.referenced_table_name}}/settings" class="hint-link">here</a>.

Copilot uses AI. Check for mistakes.
<span *ngIf="folder.name === 'All Tables'"
class="material-icons collapsed-folder-icon"
[style.color]="getFolderIconColor(folder)">dehaze</span>
class="material-icons collapsed-folder-icon">dehaze</span>

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The color styling [style.color]="getFolderIconColor(folder)" was removed from the "All Tables" icon in collapsed view (line 28), but it's still present for the same folder in expanded view (line 108). This creates an inconsistency where the "All Tables" icon will have different coloring behavior between collapsed and expanded states. Consider applying the same change to both states or documenting the intentional difference.

Suggested change
class="material-icons collapsed-folder-icon">dehaze</span>
class="material-icons collapsed-folder-icon"
[style.color]="getFolderIconColor(folder, currentCollapsedFolder === folder.id)">dehaze</span>

Copilot uses AI. Check for mistakes.

.table-list-sidenav-container ::ng-deep .mat-drawer:not(.mat-drawer-opened):not(.mat-drawer-animating) .mat-drawer-inner-container {
display: initial;
display: block;

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The change from display: initial; to display: block; may have unintended side effects. The initial value resets the display property to its default based on the element type, while block explicitly forces block-level display. Verify that this change doesn't break the layout for elements that should naturally be inline or have other display values.

Suggested change
display: block;
display: initial;

Copilot uses AI. Check for mistakes.
const url = window.URL.createObjectURL(blob);

a.href = url;
a.download = 'myFile.csv';

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

The filename 'myFile.csv' is hardcoded. Consider using a more descriptive filename that includes the table name and timestamp, such as ${this.tableName}_export_${Date.now()}.csv to make exported files more identifiable and prevent overwriting.

Suggested change
a.download = 'myFile.csv';
const tableName = this.tableName ? this.tableName : 'table';
a.download = `${tableName}_export_${Date.now()}.csv`;

Copilot uses AI. Check for mistakes.
color: #ffffff !important;
}

/* .folder-header.expanded .folder-name, */

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

There's a commented-out CSS selector /* .folder-header.expanded .folder-name, */ at line 491. This commented code should either be removed if it's no longer needed, or uncommented if it's still required for the expanded folder state. Leaving commented code in the codebase without explanation can cause confusion.

Suggested change
/* .folder-header.expanded .folder-name, */

Copilot uses AI. Check for mistakes.
Comment on lines +526 to +560
exportData() {
console.log('export data');
console.log(this.selection.selected);

// Helper function to convert value to CSV-safe string
const convertToCSVValue = (value: any): string => {
// Handle null and undefined
if (value === null || value === undefined) {
return '';
}

// Handle nested objects and arrays - convert to JSON string
if (typeof value === 'object') {
try {
// Convert object/array to JSON string
value = JSON.stringify(value);
} catch (e) {
// Handle circular references or other JSON stringify errors
value = '[Object]';
}
}

// Convert to string if not already
const stringValue = String(value);

// Check if value needs to be quoted (contains comma, double quote, or newline)
if (stringValue.includes(',') || stringValue.includes('"') || stringValue.includes('\n') || stringValue.includes('\r')) {
// Escape double quotes by doubling them and wrap in quotes
return `"${stringValue.replace(/"/g, '""')}"`;
}

return stringValue;
};

const header = Object.keys(this.selection.selected[0]);

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

The exportData() method doesn't handle the case when selection.selected is empty. If no rows are selected, accessing this.selection.selected[0] at line 560 will throw an error. Consider adding a guard condition at the beginning of the method to check if any rows are selected before proceeding with the export.

Copilot uses AI. Check for mistakes.
Comment on lines +527 to +528
console.log('export data');
console.log(this.selection.selected);

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

There are two console.log statements (lines 527-528) that should be removed before merging to production. Debug logging statements should not be committed to the codebase.

Suggested change
console.log('export data');
console.log(this.selection.selected);

Copilot uses AI. Check for mistakes.
width: calc(100% + 8px);
}

@media screen and (width <= 600px) {

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Consider using the modern CSS width <= syntax consistently throughout the codebase. While width <= 600px is valid, the more commonly used and widely supported syntax is max-width: 600px. This improves compatibility with older browsers and CSS processors.

Suggested change
@media screen and (width <= 600px) {
@media screen and (max-width: 600px) {

Copilot uses AI. Check for mistakes.
width: 100%;
box-sizing: border-box;
min-height: 0;
/* min-height: 0; */

Copilot AI Nov 17, 2025

Copy link

Choose a reason for hiding this comment

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

The CSS property min-height: 0; is commented out at line 441. If this property is no longer needed, the comment should be removed entirely. If it's temporarily disabled for testing, consider adding an explanatory comment about why it's commented out and when it should be restored.

Suggested change
/* min-height: 0; */

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants