Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor frontend code #2001

Open
3 tasks
zhoukerrr opened this issue May 7, 2023 · 4 comments
Open
3 tasks

Refactor frontend code #2001

zhoukerrr opened this issue May 7, 2023 · 4 comments

Comments

@zhoukerrr
Copy link
Contributor

zhoukerrr commented May 7, 2023

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
@ckcherry23 ckcherry23 changed the title Reorganise frontend files Refactor frontend code May 11, 2023
@jq1836
Copy link
Contributor

jq1836 commented Aug 23, 2023

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
  • 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:

  • fileTypes checkboxes (similar to c-summary's fileTypes)
  • extract SCSS styling out into its own file
  • 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):
image



From c-zoom.vue:

  • extract SCSS styling out into its own file
  • fileTypes checkboxes (similar to c-summary's fileTypes)
  • form elements (not crucial with it's current size)
  • commit result boxes can be packaged separately as a component

Screenshot of commit result boxes (for identification purposes):
image

@jq1836
Copy link
Contributor

jq1836 commented Aug 29, 2023

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:

  1. Storing data globally on existing Vuex store.
  2. 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.

@jq1836
Copy link
Contributor

jq1836 commented Sep 6, 2023

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).

  1. 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
  2. Vue component events (Pros and cons discussed earlier).

Do share any thoughts/ideas on this issue.

ckcherry23 pushed a commit that referenced this issue Feb 6, 2024
…2096)

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, starting with
extracting c-authorship-file from views/c-authorship.

This would provide a starting point towards further refactoring of the
frontend.
@vvidday
Copy link
Contributor

vvidday commented Mar 9, 2024

@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.

ckcherry23 pushed a commit that referenced this issue Mar 27, 2024
)

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, continuing with
extracting c-zoom-commit-message from views/c-zoom.
georgetayqy added a commit that referenced this issue Apr 4, 2024
* [#2120] Update RepoSense contributors in documentation (#2138)

The current About page on the RepoSense docs does not reflect the
updated list of developers working on RepoSense.

Let's move to update the list to more accurately reference the current
developers of RepoSense.

* [#2001] Extract c-zoom-commit-message component from views/c-zoom (#2132)

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, continuing with
extracting c-zoom-commit-message from views/c-zoom.

* [#2142] Fix Vulnerabilities (#2143)

Fix vulnerabilities in codebase.

There are existing vulnerabilities in the codebase.

Let's fix as many as possible.

* Bump follow-redirects from 1.15.4 to 1.15.6 in /frontend (#2160)

Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.4 to 1.15.6.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.15.4...v1.15.6)

* Bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /frontend (#2168)

Bumps [webpack-dev-middleware](https://github.com/webpack/webpack-dev-middleware) from 5.3.3 to 5.3.4.
- [Release notes](https://github.com/webpack/webpack-dev-middleware/releases)
- [Changelog](https://github.com/webpack/webpack-dev-middleware/blob/v5.3.4/CHANGELOG.md)
- [Commits](webpack/webpack-dev-middleware@v5.3.3...v5.3.4)

* [#2151] Update LoadingOverlay and Minor Versions of Node Dependencies (#2152)

Update LoadingOverlay and Minor Versions of Node Dependencies

Some dependencies are not at their latest minor or patch releases.

Let's update these dependencies, as well as LoadingOverlay as part of a
bug fix.

* Factor out markdown parser

* [#2109] Add search by tag functionality (#2167)

Add search by tag functionality

It can be useful to search author/repos by git tags.

Let's add this functionality to make it easier to filter by tags. This
commit also fixes a bug that existed in a previous version of the
feature which resulted in all users being shown to belong to same group.

* Refactor chunks

* Fix style

* Add parts of blurb

* Fix linting

* Fix style

* Fix missing html parsing

* Remove unused import

---------

Co-authored-by: George Tay <george_tay@u.nus.edu>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: jonasongg <120372506+jonasongg@users.noreply.github.com>
ckcherry23 pushed a commit that referenced this issue Apr 23, 2024
…oom (#2173)

Extract c-file-type-checkboxes from Summary, Authorship and Zoom

The checkbox collection is duplicated across many components.

Let's abstract them into a separate file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

4 participants