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

[WIP] refactor: app state + widget refactor #488

Closed
wants to merge 1 commit into from

Conversation

ClementTsang
Copy link
Owner

@ClementTsang ClementTsang commented May 24, 2021

Description

A description of the change and what it does. If relevant (such as any change that modifies the UI), please provide screenshots of the change:

Rather large and long-awaited refactoring of application state to not be as bloated and/or hard to develop for.

Main goals:

  • Shift GUI element state away from main app state, should be stored within the element
  • Maybe look into rewriting how GUI is laid out + stored in state to reduce stuff like widgets.get_mut(blah blah blah...), that gets real tiring real quick.
  • This may also require refactoring/rewriting how we do element layouts and event handling.

Current ideas/thoughts on implementation:

  • Most of the current app state is actually fine. Storing things like collected data, converted data, if we're in dd - these are all valid things to store in state. Maybe clean/group it up a bit, but that's pretty low priority.
  • The main things I want to get rid of in the app state is all the GUI element state stuff. Stuff like "scroll offset" or "cursor offset" - these shouldn't be stored in the app state as they are now. It causes this horrible interconnected mess that makes expanding really, really hard, and is the main reason I haven't been willing to even touch things like config.
  • What this also entails, then, is a refactoring of how we do GUI. Currently, we have this questionable (I have no idea what I was writing) Painter class that... implements traits to draw specific elements. This is awful.
    • There is so much code dup. Our widgets are mostly just table/graphs, for example. But for those tables and graphs, each one requires a separate implementation, where 80% of the code is the same!
    • We currently store zero state in the elements. We should. It makes no sense to tie it inside app state.
    • We should also ideally move how these elements handle inputs. Ideally, we can have the option to delegate input handling within widgets if global shortcuts cannot handle them.

Issue

If applicable, what issue does this address?

Affects (maybe closes) #374, #234

Testing

If relevant, please state how this was tested. All changes must be tested to work:

Furthermore, mark which platforms this change was tested on. All platforms directly affected by the change must be tested

  • Windows
  • macOS
  • Linux

Checklist

If relevant, ensure the following have been met:

  • Areas your change affects have been linted using rustfmt (cargo fmt)
  • The change has been tested and doesn't appear to cause any unintended breakage
  • Documentation has been added/updated if needed (README, help menu, etc.)
  • The pull request passes the provided CI pipeline
  • There are no merge conflicts

@ClementTsang ClementTsang changed the title [WIP] refactor: app state refactor [WIP] refactor: app state + widget refactor May 26, 2021
@ClementTsang ClementTsang force-pushed the state_refactor branch 6 times, most recently from a0feabc to 870ae64 Compare June 12, 2021 22:53
@ClementTsang ClementTsang force-pushed the state_refactor branch 3 times, most recently from e6b22a9 to 01d683e Compare July 4, 2021 02:23
@ClementTsang ClementTsang mentioned this pull request Jul 4, 2021
8 tasks
@ClementTsang ClementTsang force-pushed the state_refactor branch 2 times, most recently from 3b09b9b to 0eefac5 Compare July 13, 2021 03:45
@ClementTsang ClementTsang force-pushed the state_refactor branch 6 times, most recently from de86ceb to 6ecf1de Compare July 16, 2021 01:33
refactor: Create a prototype time graph element

Move some state content over

refactor: rename folder

refactor: More renaming and pruning of old unused code.

Create a prototype time graph element

scrollable table

Temporarily add dead code allows

Minor refactorings

refactor: Move current widgets to the new element system

squash this - WIP of re-refactor

Revert "squash this - WIP of re-refactor"

This reverts commit a3ce25732663863b1ba2ef8a764c57572406014f.

Revert "refactor: Move current widgets to the new element system"

This reverts commit c7218472c2ce6d1bebf5550aaf53c171d3cd7c3e.

WIP re-refactor

Revert "refactor: Create a prototype time graph element"

This reverts commit bd95ca8.
@ClementTsang
Copy link
Owner Author

Closing this for now until it's closer to being ready.

@ClementTsang ClementTsang mentioned this pull request May 16, 2022
15 tasks
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

Successfully merging this pull request may close these issues.

1 participant