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 UI to use common components #56

Open
ValerianClerc opened this issue May 23, 2020 · 2 comments
Open

Refactor UI to use common components #56

ValerianClerc opened this issue May 23, 2020 · 2 comments

Comments

@ValerianClerc
Copy link
Member

No description provided.

@AsFal
Copy link
Member

AsFal commented Aug 21, 2020

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

  • Form submission
  • Title
  • Fields

[REFACTOR] About page Timeline items

[REFACTOR] First Page Announcements

ETC.

Require access to an account to further investigate.

MISC

  • [REFACTOR] Clean up magic strings (eventBus event names, action names, url, etc.)
  • [REFACTOR] Clean up text in html using i18n (as a bonus, translating the website would require minimal effort)
  • [REFACTOR] move scheduling to backend
  • [REFACTOR] Remove useless mixins folder
  • [REFACTOR] Remove useless filters folder

@ValerianClerc
Copy link
Member Author

ValerianClerc commented Aug 24, 2020

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)

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

No branches or pull requests

2 participants