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

[#2001] Extract SCSS from c-zoom.vue #2032

Closed
wants to merge 5 commits into from

Conversation

jq1836
Copy link
Contributor

@jq1836 jq1836 commented Aug 29, 2023

Part of #2001

Proposed commit message

Currently, frontend code for c-zoom.vue is especially hard to read and is in
need of refactoring.

Let's extract SCSS and package related logic together to reduce the size of
c-zoom.vue and make it more readable.

Other information

Tasks to be done before PR is ready:

  • Extract SCSS
  • Repackage fileType checkboxes as vue component
  • Repackage daily commit result boxes as vue component

@jq1836 jq1836 marked this pull request as draft August 29, 2023 05:54
@jq1836
Copy link
Contributor Author

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, we should do so via Solution 2.

@jq1836 jq1836 changed the title Extract SCSS from c-zoom.vue [#2001] Extract SCSS from c-zoom.vue Aug 31, 2023
@jq1836
Copy link
Contributor Author

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.

@jq1836
Copy link
Contributor Author

jq1836 commented Sep 13, 2023

PR is not moving forward as of now, more discussions to be made at #2001 before pursuing again.

@jq1836 jq1836 closed this Sep 13, 2023
@github-actions
Copy link
Contributor

The following links are for previewing this pull request:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants