kblincoe / VisualGit_SE701_2019_1

1 stars 0 forks source link

Change all usages of document to use angular template binding #151

Open bcox280 opened 5 years ago

bcox280 commented 5 years ago

Description

Currently, we are getting all elements from the document, by querying for class or querying for IDs. This is realllllly bad as whenever there is a class or id that is not unique, you will get weird behaviour. Also, you need a null check every time you pull an element out of the document. There also may be clashes on ids if people are using similar ids on elements.

Acceptance Criteria

All usages of the document should be removed and replaced with angular binding. Angular binding is a way of assigning and getting values from elements in a more angular fashion. This is safer and in the future will allow people to more easily access document elements.

https://angular.io/guide/template-syntax

bcox280 commented 5 years ago

With this issue, would also require moving any styling done to be done within the html/scss. An example of a case where this would be applicable would be in the git.ts where files are being assigned to the dif checker in displayModifiedFile() method. Instead of having it so each element is created in the typescript, and done programmatically, you use an Angular binding instead, which would separate out the responsibilities/concernes. This way, using an Angular directive and interpolation, you would inject the data into the components, and the directive would handle making multiple elements. https://angular.io/guide/displaying-data view this link to understand this further

aorthi commented 5 years ago

I approve, this is something that would not only makes the project follow Angular standards, it also would make it much easier to be picked up by people in the future.

KelseyRM commented 5 years ago

Approved, this would definitely make the code a lot cleaner and safer to access document elements

nathan-cairns commented 5 years ago

Approved