search

competemcgill / wisp-ui

WISP User Interface
https://wisp.training
MIT License
3 stars 2 forks source link

Refactor UI to use common components #56

Open ValerianClerc opened 4 years ago

AsFal commented 4 years ago

Wisp UI refactor proposals

Views

[REFACTOR] Using subrouters for Views

Problem

Views are currently built rendered from index files nested in their associated subfolders in the Views directory. Sub Views (see example below) are defined by adding subfolders containing index files to the view folders. 

Problems
├── Index
│   └── index.vue
└── Show
    └── index.vue

Improvement

By leveraging subrouters, subviews could be grouped together without creating having to create a folder every time. This technique allows the developper to define a wrapper in the Index.vue component which will be present visible in each implemented page. On top of that, route guards could be defined for the subrouters.

Problems
├── Index.vue
└── pages
    ├── ProblemsShow.vue
    └── ProblemsIndex.vue

[REFACTOR] Scoping single use components to their views

Problem

The components folder currently contains a multitude of single use components that are tightly coupled to their associated view. Defining the component away from the view created uneccessary distance between connected pieces of code. For example, the Admin/ProblemForm component is only used in the Admin view.

Improvement

Components that are simply a section of a view should be defined in the same folder as the index file for the view. The components folder should be used for generic "container" components that are reused multiple times in the code to ensure that the entire ui has the same feel. For example, the Admin view would be modified to the following file structure.

Admin
├── Index.vue
└── components
    ├── ProblemForm.vue
    └── ProblemSetForm.vue

Store

[REFACTOR] Modules

Problem

As it stands right now, the store contains very few actions and mutations. This could rapidly change as the application scales up and make the actions.js and mutations.js files large and hard to manage.

Solution

Seperate actions, mutations and state by module. https://vuex.vuejs.org/guide/modules.html

[REFACTOR] API Client

Problem

The gateways folder seems like something that is there to confuse a new developper. All it does is exposes an api object which can be used to communcate with the wisp gateway. Frontend code shouldn't be named based on backend architeture features.

The api exposed by gateways also has no builtin functionality. As a result, wisp-api client functionalities are spread across the entire application.

Solution

Individual components should not be allowed to communicate directly with the wisp api. Interactions should be done through store functionality (either the eventbus or store actions, just not a mix). This breaks the MVVM architeture which inspired the Vue framework, given that ViewModels (Component scripts) contain Model logic (instructions on how to interact with the data contained in the API).

Actions

This solution would require an overhaul of the eventbus.

The following article explains the pattern better than I can. [https://medium.com/js-dojo/vuex-2638ba4b1d76](Vuex - how to use state).

Essentially, api interactions are abstracted by actions. Components are only allowed to dispatch actions and react to state change.

Eventbus

My understanding is that the goal of the eventbus is to implement to Observer-Observable pattern. My proposal will thus be inspired from documentation written by https://rxjs-dev.firebaseapp.com/guide/overview, which would also be a better suited library for using the pattern instead of using a simple Vue object.

Currently, the eventbus implements a https://rxjs-dev.firebaseapp.com/guide/subject with multiple events. My proposal is to simply define the api interactions in the same place the eventbus is defined. The components could then use these interactions without having to actual implement model logic themselves.

Generic components

[REFACTOR] Generic Form Component

Incosistent form style (see register view vs. login view). Create generic component which allows the user to specify props for

MISC

ValerianClerc commented 4 years ago

Thanks @AsFal for writing up this detailed proposal. I'm on board with all of the refactors that you proposed in detail. Feel free to get started on this anytime, and let me know if you need explanations on any of the codebase parts.

The dev cluster is currently down because summer training is over (and we're out of free credits for hosting for now). So to test things until we rehost the site, please use these scripts, namely make start-api to spawn the API microservices locally. Then you can run the wisp-ui repo locally for development, making it point to localhost:3000. Ping me or @MohamedBeydoun if you have issues getting set up.

(Also @MohamedBeydoun please review this proposal and object if you have any problems)