Skip to content

Conversation

@colinthebomb1
Copy link
Contributor

@colinthebomb1 colinthebomb1 commented Aug 12, 2025

Purpose

This PR adds a file editor feature for datasets, enabling users to create, edit, and manage Markdown and text files directly within the dataset interface. The component makes it easy to extend the functionality to new file types in the future (CSV). The files are stored as versioned files within the dataset using the existing FileService, ensuring proper version control. It also provides an easy way to create a README file directly from the dataset versioning tab

closes #3517
Design document

Changes

  • Created a new UserDatasetFileEditorComponent that:
    • Allows authorized users to edit, and save the markdown and text files.
    • Provides a markdown editor using angular-markdown-editor for md files and a basic text box for txt files
    • Easily extensible to new file formats in the future (CSV).
  • Added easy access to README creation if it's not present in the dataset.
  • Use existing FileService for create/update actions and file version control.

Demo:

demo.mp4

@bobbai00 bobbai00 self-requested a review August 14, 2025 02:29
@bobbai00 bobbai00 changed the title feat: add dataset file editor feat(dataset): add dataset file editor Aug 14, 2025
Copy link
Contributor

@bobbai00 bobbai00 left a comment

Choose a reason for hiding this comment

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

Good start, left some comments

</button>

<!-- README creation form -->
<div *ngIf="showReadmeForm">
Copy link
Contributor

Choose a reason for hiding this comment

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

you should try to reuse the file editor component, a separate form seems to be redundant

-->

<!-- Loading State -->
<div *ngIf="isLoading" class="file-loading">
Copy link
Contributor

Choose a reason for hiding this comment

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

Few high level comments regarding this component:

  1. Can you merge it with the file-renderer component: if a file is editable,
    Screenshot 2025-08-13 at 11 44 24 PM show an edit button. And when users click that button, show the editing interface. This can avoid the redundancy file header.

  2. I still want you to explore the library way of having the markdown editor. Implementing one by ourselves brings high maintenance cost and can be error-pruning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@bobbai00 bobbai00 left a comment

Choose a reason for hiding this comment

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

Left some comments

"backbone": "1.4.1",
"bootstrap": "^5.3.7",
"bootstrap-markdown": "^2.10.0",
"content-disposition": "0.5.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix the versions of the libraries you introduced by removing ^

"html2canvas": "1.4.1",
"jointjs": "3.5.4",
"jquery": "^3.7.1",
"js-abbreviation-number": "1.4.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Move it to the dependee package, and leave a comment

"es6-weak-map": "2.0.3",
"file-saver": "2.0.5",
"font-awesome": "^4.7.0",
"fuse.js": "6.5.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

"@types/graphlib": "2.1.8",
"@types/jasmine": "4.6.4",
"@types/jquery": "^3",
"@types/json-schema": "7.0.9",
Copy link
Contributor

Choose a reason for hiding this comment

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

@aglinxinyuan Please take a look to see if both devDependencies & dependencies need to have these packages

chunkSizeMB: number = 50;
maxConcurrentChunks: number = 10;

public isCreatingReadme: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this

.pipe(
switchMap(blob => {
return new Promise<string>((resolve, reject) => {
const reader = new FileReader();
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace Promise style with rxjs style

this.fileExists = false;
this.fileContent = "";
this.editingContent = "";
console.log("File not found or error loading");
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this log

}
}

public getFileName(): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this, and test the editing of a file in folders


public fileContent: string = "";
public fileExists: boolean = false;
public isLoading: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

A general comment: see if you can remove some of the state variables

}

public isEditable(): boolean {
return this.fileType === "markdown" || this.fileType === "text";
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this

@github-actions github-actions bot added dependencies Pull requests that update a dependency file frontend Changes related to the frontend GUI labels Oct 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Need README Page for Datasets

3 participants