Feature: file explorer tabs #701
Conversation
| @@ -49,6 +50,7 @@ export class EditorComponent implements ControlValueAccessor, AfterViewInit, OnC | |||
| public placeholder: string; | |||
| private _value = ""; | |||
| private _sub: Subscription; | |||
| private _erd: any; | |||
There was a problem hiding this comment.
what is an erd? :) maybe call it _resizeDetector, i am a firm believer of descriptive variable names. Particularly helpful for junior devs.
There was a problem hiding this comment.
yeah was the same name in the heatmap and copied it from there. But can change it
| .tablist { | ||
| height: $tab-height; | ||
| // border-bottom: 1px solid $border-color; | ||
| background: #e5e5e5; |
There was a problem hiding this comment.
need to put #e5e5e5 into variables. oh, its there. change to $mercuryGray
| constructor(data: FileNavigator | FileSource | FileSource[]) { | ||
| this.sources = sourcesFrom(data); | ||
| this.openedFiles = this._openedFiles.asObservable(); | ||
| // this.openedFolder = this._openedFolder.asObservable(); |
There was a problem hiding this comment.
is this going to be used in the future? otherwise delete:
public openedFolder: Observable
There was a problem hiding this comment.
forgot to remove it
| public goBack() { | ||
| const path = this._currentPath.value; | ||
| if (path === "") { return; } | ||
| this.navigateTo(path.split("/").slice(0, -1).join("/"), this._currentSource.value); |
There was a problem hiding this comment.
you have used ".split(/[\/]/)" in the past, do we need to do that here to take into account / or \
There was a problem hiding this comment.
the path is already suppose to be normalized in the tree though
There was a problem hiding this comment.
but there is a util method dirname I made I should use here instead
| if (this.sources.length === 1) { | ||
| return this.sources.first(); | ||
| } else { | ||
| log.error("You must specify a source(FileNavigator) when using multi-source"); |
There was a problem hiding this comment.
bit of a nitpick but this could go into a const so we only need to change it in one place if needs be.
There was a problem hiding this comment.
yeah speaking of that I think we should look at internationalization that would make the code cleaner for long messages at least
| } | ||
| > .file-icon { | ||
| > .fa-folder, > .fa-folder-open { | ||
| color: #f8d979; |
| <bl-file-content [fileLoader]="fileLoader" *ngIf="fileLoader"></bl-file-content> | ||
| </div> | ||
| <div *ngIf="fileNotFound" class="info-overlay file-not-found"> | ||
| File <span class="highlight"> {{filename}} </span> doesn't exists. |
| @Input() public jobId: string; | ||
|
|
||
| @Input() public taskId: string; | ||
| @Input() public task: Task; | ||
|
|
||
| public OutputType = OutputType; |
There was a problem hiding this comment.
no this is to pass the enum to the template. I think it is better to keep the same case
There was a problem hiding this comment.
oh ok cool. if you are happy then i am happy :)
| { | ||
| name: "Node files", | ||
| navigator: nodeNavigator, | ||
| openedFiles: ["stdout.txt", "stderr.txt", "wd/example-jobs/python/pi.py"], |
There was a problem hiding this comment.
"wd/example-jobs/python/pi.py" ?
There was a problem hiding this comment.
oops forgot to remove that one
| if (!change) { return false; } | ||
| const { previousValue, currentValue } = change; | ||
| const same = previousValue && currentValue && previousValue.id === currentValue.id; | ||
| return !same; |
There was a problem hiding this comment.
probably just need:
return previousValue && currentValue && previousValue.id === currentValue.id;
There was a problem hiding this comment.
doing a ! here this would be the opposite
| expect(storageServiceSpy.navigateContainerBlobs).toHaveBeenCalledOnce(); | ||
| expect(storageServiceSpy.navigateContainerBlobs).toHaveBeenCalledWith("job-1", "task-1/", jasmine.anything()); | ||
| })); | ||
| }); |
There was a problem hiding this comment.
Have we make sure that the file size reloads from '-' to the actual size when the file becomes available. as per #523. if that's fixed then include this issue in the PR so it gets closed.
Added tabs to the file explorer. This allows to merge the task logs into the file explorer
fix #655
fix #689
fix #523(File size doesn't reload)
Show better message when task has not run yet
Tabs!
