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 Components and Pages #31

Closed
Keelando opened this issue Dec 4, 2021 · 8 comments
Closed

Refactor Components and Pages #31

Keelando opened this issue Dec 4, 2021 · 8 comments
Assignees

Comments

@Keelando
Copy link
Member

Keelando commented Dec 4, 2021

Many of the components will need to be refactored into smaller chunks and individual components for readability and easier editing and debugging.

For example, the component called LatticeView.tsx is very big and can be separated into smaller components.

This will probably occur "soon-ish" after more web UI functionality is brought in.

@alexnguyenn
Copy link
Member

Bumping this - I think we should prioritize this first.

@alexnguyenn
Copy link
Member

alexnguyenn commented Dec 20, 2021

@gwwatkin some ideas from my last discussion with @Keelando:

Splitting LatticeView should be the main concern here. I am thinking of something like this:

components/
├─ LatticeView/
│  ├─ Cell.module.css
│  ├─ Cell.tsx
│  ├─ Compilation.module.css
│  ├─ Compilation.tsx
│  ├─ Lattice.module.css
│  ├─ Lattice.tsx
│  ├─ LatticeView.module.css
│  ├─ LatticeView.tsx

We would have LatticeView as the parent component of Compilation and Lattice (which is the parent of Cell).

Also we should limit the usage of emotion (the kinda inline CSS) in our code unless it is necessary (I guess for dynamic styling). I recommends having styling for a component stored in the *.css file of the same name (also recommend using CSS modules as well).

@gwwatkin
Copy link
Member

These are all great ideas and definitely desirable features. Also agree with the file structure. Only thing I can suggest is LatticeView -> CompilationView looking to turn the compilation text into proper views, lattice would just be one of them

@gwwatkin
Copy link
Member

I think refactoring is important, but would like to mention that I think it's important to keep an eye on what creates value for the project. Some times it's easy to get caught up in making things look pretty in the code and loose connection with what matters for the user

@gwwatkin
Copy link
Member

gwwatkin commented Dec 20, 2021

What I propose is that we conclude the current PR and implement the remaining changes along the way while moving forward on other UI issues. When refactoring a lot before making new changes there is risk of doing and undoing work - we want to be strategic an refactor things as they improve our productivity

@alexnguyenn
Copy link
Member

@gwwatkin agree that we should implement those changes alongside with working on UI stuffs. The main goal here is to have enough separation so that it is easier to manage and update later on.

@alexnguyenn
Copy link
Member

Just on the progress for this issue: I think the current separation that we are having is good, the last component I would split is CompilationText from LatticeView.

@gwwatkin
Copy link
Member

gwwatkin commented Mar 3, 2022

Components are nicely formatted now, starting from:

@gwwatkin gwwatkin closed this as completed Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants