Closes #48 - Built in Editor

Buster-Darragh-Major commented 5 years ago

Related Issue/Keyword:

Closes #48

Description:

A new file explorer component has been added the the right hand side of the graph on the main screen. A user can navigate through files and select files they wish to view/edit. Upon clicking a file they are presented with a text editor view where they can make alterations to the file, save them and navigate back.

@Jess-Alcantara I see you've referenced this issue? Everything cool?

Known Issues:

Because of the current configuration with watchman (which detects changes in files and reloads the app accordingly), if a user makes clones and makes changes to a file within the VisualGit_SE701_2019_1 repository, watchman detects these changes thinking they are part of the source code, recompiles and reloads. A separate issue should probably be opened for this.
There are an unreasonable amount of linting issue with this code (I'm talking 100+) so I am searching for a way to fix them all automatically before this is merged

Testing:

If your changes cannot be tested programmatically, add steps below so that the reviewer can manually tests your changes. Otherwise, list the names of the new unit tests you have added to the test suite.

Steps for manual testing:

Clone a repository and navigate to it within the app
Select a file through the r.h.s. file explorer. Make sure to try both files, directories and the back button.
Select a file to edit, make an edit to this file, save it (under current issue described above the app should reload).
Navigate back to the file you changed and verify that the change made is still present.

Newly added unit tests:

No unit tests as no unit testing framework

Checklist:

[x] Latest master merged/rebased into your feature branch
[] Tests covering all changes
[x] Meets the projects coding conventions
[x] No out of scope changes
[x] @Mentioned any relevant team members
[] No failure when running the linter (npm run lint)
[x] Included a relevant gif (search here for inspiration)

Buster-Darragh-Major commented 5 years ago

Have managed to automatically fix most of the linting errors so I'll add the linting fixes shortly

Buster-Darragh-Major commented 5 years ago

Have managed to automatically fix most of the linting errors so I'll add the linting fixes shortly

All remaining (un-automatically fixable) linting errors are from files I have not touched so in the interests of issue scope I have left them

Buster-Darragh-Major commented 5 years ago

Issue Added Complexity

I first attempted importing external directory tree component libraries (ng-file-tree, ng-tree-component and ng2-directory-tree) for navigating the directory tree with little success. Several fellow team members were also unable to solve the issue. Hence, this issue had the added overhead of building the directory navigation component. This first stage ate up quite a lot of time.

Secondly, because of the current awkward architecture of the application with a large amount of code being found in global functions, there was a lot of overhead to understanding where variables were set and document elements were manipulated which was also not consistent across the code base. This also meant that acquiring a reference to the ProjectPanelComponent object in the repo.ts file was very difficult to figure out given the unorthodox design and lack of experience I have in Angular. This stage also took a large amount of time.

darcycox97 commented 5 years ago

@Buster-Darragh-Major looks like you have a cheeky couple conflicts. I'll review when those are sorted out 💯

Buster-Darragh-Major commented 5 years ago

@darcycox97 try that

darcycox97 commented 5 years ago

Functionality mostly looking good! However, found an issue where if you navigate out of the current repository and click back too far, an exception is thrown, and after this point I can't navigate back down the tree. Error is shown in image below:

Perhaps it would be best to not let users navigate out of the current repository, for safety purposes.

Feel free to make another issue for this functionality if you don't think it should be included in this PR

darcycox97 commented 5 years ago

Hmm that's really bizarre. Using import statement definitely didn't work for me but require('path') fixed it. Let me have a look in the morning when I'm more awake. If we cant get it working we could just get a mac user to confirm it's allgood

On Sat, 6 Apr 2019, 11:48 pm Buster Darragh-Major, notifications@github.com wrote:

@Buster-Darragh-Major commented on this pull request.

In app/services/projectDirectory.service.ts https://github.com/kblincoe/VisualGit_SE701_2019_1/pull/233#discussion_r272792023 :

@@ -0,0 +1,62 @@ +import { Injectable } from '@angular/core'; +import Git = require('nodegit'); + +@Injectable() +export class ProjectDirectoryService { +

currentDir: string;

fileSep: string;

constructor() {

if (process.platform === 'win32') {

this.fileSep = '\';

@darcycox97 https://github.com/darcycox97 This is the error I get when I do that, weird because I definitely have node installed as well. The path it's looking for is interestingly different from before as now its just searching for VisualGit_SE701_2019_1/path instead of VisualGit_SE701_2019_1/node_modules/path which it was doing before, is there something special I have to do with my project to let it know about node's existence? [image: Capture] https://user-images.githubusercontent.com/26570703/55668404-79b22e00-58c6-11e9-86ce-08bb0887b0f9.PNG

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kblincoe/VisualGit_SE701_2019_1/pull/233#discussion_r272792023, or mute the thread https://github.com/notifications/unsubscribe-auth/AdkChOYd6AX7llM_Da0cZzu9Bsoxz0m-ks5veHuTgaJpZM4ceGYV .

Buster-Darragh-Major commented 5 years ago

@darcycox97 and @cyrus-raitava if that's ll good then now we cool to merge?

kblincoe / VisualGit_SE701_2019_1