RepoSense
RepoSense copied to clipboard
Refactor frontend code
What feature(s) would you like to see in RepoSense
Many of the frontend files are difficult to navigate through due to the large file sizes. As we add more features to the frontend, it is getting harder to maintain. This is also very unfriendly towards new contributors. Let us break down frontend files in a logical manner.
Here are some of the possible steps:
- [ ] Refactor code into smaller parts
- [ ] Extract CSCC styling out and see if we can reduce code duplication
- [ ] Update documentation after refactor
Agreed with how increasingly difficult it is to navigate large files and identify which part of the frontend each part of the code affects. Looking through the frontend codebase, I've picked out the main offenders of this, being c-summary.vue
, c-authroship.vue
and c-zoom.vue
. After looking through my suggestions, do share your opinions on this!
Some suggestions as to which files we could break down into smaller parts are as follows:
From c-summary.vue
:
- [ ]
summary-picker
form logic to facilitate future additions to the form - [x]
fileTypes
checkboxes can be packaged separately as a component due to its use in the summary, zoom and authorship view - [ ]
error-message-box
(not crucial with it's current size)
From c-authorship.vue
:
- [x]
fileTypes
checkboxes (similar to c-summary'sfileTypes
) - [ ] extract SCSS styling out into its own file
- [x]
files
commit authorship box can be packaged separately as a component - [ ] form elements (not crucial with it's current size)
Screenshot of a single commit authorship box (for identification purposes):
From c-zoom.vue
:
- [ ] extract SCSS styling out into its own file
- [x]
fileTypes
checkboxes (similar to c-summary'sfileTypes
) - [ ] form elements (not crucial with it's current size)
- [x] commit result boxes can be packaged separately as a component
Screenshot of commit result boxes (for identification purposes):
While trying to create a general file type checkbox component that can be used in c-zoom.vue
, c-authorship.vue
and c-summary.vue
, reducing repeating of code, issues regarding the propagation of data from child components to parent components arose.
To resolve the issue with propagating data from child to parent, the following are possible solutions:
- Storing data globally on existing Vuex store.
- Making use of Vue component events.
Solution 1 vs Solution 2:
- Solution 1 is already being used to transmit application data globally.
- Solution 1 clutters the global Vuex store and further couples the store with components using it.
- Solution 2 couples the parent and child components, and anything in between.
Personal opinion:
Due to the proximity of the c-zoom.vue
, c-authorship.vue
and c-summary.vue
with a potential implementation of file type checkbox component, in this case due to the parent component being composed of the child component, the coupling introduced will be far less significant compared to coupling the child component with a global store. Hence, if we are to proceed with repackaging the file type checkbox and in future when splitting larger components into smaller ones.
EDIT:
After further thinking, I think that standardising everything to make use of Vuex would make more sense, especially with the fact that we are able to nest compound types in the Vuex store. Will proceed with implementing as such.
While trying to implement the Vuex store based solution, I experienced difficulty making the file type checkboxes reusable. The following are the options for implementing the global store based solution that is reusable (the main point of this PR).
- Pass relevant store mutation functions through props.
- Has to be done in the parent's scope
- Goes against observer design pattern
- Relatively simple to implement
- Vue component events (Pros and cons discussed earlier).
Do share any thoughts/ideas on this issue.
@sopa301 Feel free to continue working on this issue - regarding the checkboxes, I'm more in favour of just using component events as it seems the checkbox component would just be passing data with its direct parent, but I'm OK with both approaches if you prefer the Vuex approach.