refactor: state refactor #571
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note to self: don't squash this. |
Write some glue code to transition from the old code to the new one.
Even more glue code to help glue together our layout options to our new layout system! Note that this PR will most likely likely break the two options: - default_widget_type - default_widget_count and as such, they'll probably be deleted in a later commit. As for why, it's since they're kinda a pain to support and don't work well. Users can still enable default widget selection through the layout system (which will also see a revamp in the future).
This rips out this weird trait system I previously used for drawing widgets, where I implemented a trait onto the Painter struct that did the drawing. I have no idea what I was thinking back then.
In particular, moving over table-style widgets
Things working as of now: - Actually drawing - Interpolation - Styling
ClementTsang
force-pushed
the
state_refactor
branch
from
278c8a2
to
2bff04d
Compare
Slightly move around the ideas of EventResult, ReturnSignalResult, and how they all work. The gist of it is that we now have widgets returning EventResults (and renamed to WidgetEventResult), and the main app event handler returns ReturnSignalResult (now named EventResult). Also add a new signal to handle re-updating data inputs! This is needed for the process, and any sortable/configurable widget.
There were three bugs: 1. The click bounds calculation was incorrect. I did the silly mistake of checking for <= bounds for the bottom and right sections of a Rect when checking if the mouse intersected - this is WRONG. For example, let's say you want to calculate if an x value of 5 falls between something that starts at 0 and is 5 long. It shouldn't, right? Because it draws from 0 to 4? But if you just did <= Rect.right(), you would get a hit - because it just does (start + width), so you get 5, and 5 <= 5! So, easy fix, change all far bounds checks to <. 2. The second bug is a mistake where I accidentally did not include bounds sets for my memory and net widgets. Instead, they set their bounds to the underlying graph representation, which is WRONG, since that bound gets updated on draw, and gets set to a slightly smaller rect due to borders! 3. A slightly sneakier one. This broke my bounds checks for the CPU widget - and it would have broken my process widget too. The problem lies in the concept of widgets that handle multiple "sub"-blocks internally, and how I was doing click detection internally - I would check if the bounds of the internal Components were hit. Say, the CPU, I would check if the internal graph was hit, then if the internal table was hit. But wait! I said in point 2 that a graph gets its borders updated on draw to something slightly smaller, due to borders! And there's the problem - it affected tables too. I was setting the bounds of components to that of the *internal* representation - without borders - but my click detection *needed* borders included! Solution? Add another trait function to check bordered bounds, and make the default implementation just check the existing bounds. For cases like internal Components that may need it, I add a separate implementation. I also switched over all border bounds checks for Widgets to that, since it's a bit more consistent.
ClementTsang
force-pushed
the
state_refactor
branch
from
539ff4b
to
3fa5060
Compare
Because writing your own layout system and management is just *so much fun*. Totally. ------------------------------------------------------------------- Moves the basic mode system over to the new drawing/widget system. In the process, it has forced me to completely redo how we do layouts... again. This is because basic mode has widgets that control their own height - this means the height of the columns and rows that wrap it are also affected by the widget's height. The previous system, using a constraint tree and splitting draw Rects via tui-rs' built-in constraint solver, did not support this concept very well. It was not simple to propagate up the widths/heights towards parents while also using tui-rs' built-in constraint solver. In the end, it was easier to just rewrite it using another algorithm. We now follow a process very similar to Flutter's layout system. Relevant links to the Flutter docs are found in the code or below: - https://flutter.dev/docs/development/ui/layout/constraints - https://flutter.dev/docs/resources/inside-flutter#sublinear-layouts The gist of it, however, is that we now instead a few new options for any element in the layout tree. A node can either: - Grow to fill remaining space - Take up as much room as its children - Be a specific length Technically right now, it's not perfect, in that leaf nodes can be as large as their children (which makes no sense), though in that case it just treats it as an expand.
ClementTsang
force-pushed
the
state_refactor
branch
2 times, most recently
from
368aa2c
to
81fd703
Compare
Also has a few clippy fixes and bug fixes: - Fix redundant rerendering on scroll on time graphs. - Fix being off by one cell during rendering for no-battery situation on the battery widget. - Fix having empty columns for rtl column width calculations (as otherwise it goes to the wrong column). - Fix rendering issue on small windows with text tables. We also now ensure that the CPU legend has enough room to draw!
ClementTsang
force-pushed
the
state_refactor
branch
from
81fd703
to
9ef38cb
Compare
Adds back some of the general program keybinds, and fixes both a bug causing widget movement via keybinds to be incorrect, and not correcting the last selected widget in the layout tree rows/cols after clicking/setting the default widget!
ClementTsang
force-pushed
the
state_refactor
branch
2 times, most recently
from
7a8bbb7
to
89f54c1
Compare
Mostly previously re-added files during the merge conflict resolution, and a lot of unused code. Still more to delete after I finish rewriting the process kill dialog.
ClementTsang
force-pushed
the
state_refactor
branch
from
89f54c1
to
9089231
Compare
ClementTsang
added
the
refactor
Mouse control on collapse is not working yet, need to do some work internally first.
ClementTsang
force-pushed
the
state_refactor
branch
from
c57ef9d
to
cc66f1f
Compare
15 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
Alright, let's take another crack at this.
The goals of this refactor
Widgets orComponents themselves, barring global shortcuts, which will be handled separately.The logic I've switched to is more akin to that of i3wm - instead of manually computing on startup where to move/reflect movement actions to based on computed widget size (which was horrible), there's an easier way to do it on-the-fly with a row/column system. Note this does mean the behaviour is now different and thus breaking.Going to go back to the old behaviour, but with the new layout system.LayoutandWidgetsystem.Changes
default_widget_countanddefault_widget_typefrom configs and flags.To do
TextInputcomponentCarouselcomponent%not shift?pub fnfor now).Issue
If applicable, what issue does this address?
Closes: #234
Closes: #374
Closes: #562 (due to bumping crossterm version)
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
Checklist
If relevant, ensure the following have been met:
cargo fmt)README.md, help menu, etc.)