Simplify and Enhance Spin Button Widget #720
Conversation
|
Make sure to configure your editor to automatically run |
I just set that up, thank you. It's been fixed and pushed up. |
|
|
|
Interesting, I made a small change, saved, and changed it back to have it run cargo fmt again. I've committed and pushed. I hope that that works. |
|
Thanks! |
|
Note: had to fix widgets being named |
|
Removed the rollover behavior. It should be clippped, or otherwise implementation-defined by the user of the widget. |
|
Thank you for accepting this! I really appreciate it. I'll put it on my list to add a setter or something to add the rollover feature since it's something I personally needed when I first needed to use a vertical spin button. Not a high priority at the moment though. |
|
Need to revert to fix other issues |
| { | ||
| /// The label that the spin button widget will have. | ||
| /// It is placed on the top of and centered on the spin button widget itself. | ||
| label: String, |
| .into(), | ||
| text::title4(label) | ||
| // Using the title4 variant of text for consistency. | ||
| text::title4(format!("{}", spin_btn.value)) |
There was a problem hiding this comment.
Should be label, not the value. User must have control over formatting of the value.
There was a problem hiding this comment.
I'm not sure I understand this ask here. Is the label in this case a text representation of the value? I took it as the title of what the spin button is for.
So currently, as implemented, the label is on the top center of the widget. Whereas if it is supposed to be a formatted text representation of the current value, I misinterpreted the intent.
If that's the case I'll definitely refactor to make it the right way. I may change the parameter name from label to something else for better clarity of what it's supposed to be though.
|
I'll look into fixing the issues |
|
Are they something I can help with fixing? |
Purpose
The purpose of this PR is to simplify the Spin Button widget's API for future external COSMIC developers by removing the external Model struct that the previous version had. The Model struct made it to where the developer had to keep track of the functionality, initialization, and value updating outside of the Spin Button widget itself.
It also is to expand on functionality as an Orientation enum was added to the API so that developers can choose between a Horizontal (default) or Vertical orientation.
Spin Button Widget Improvements
The improvement over the previous implementation, although a breaking change, has all of the functionality of the Model struct self-contained within the Spin Button widget itself.
This will allow for developers who are using the library to no longer have to keep track of the creation of the Spin Button widget and the functionality through an external Model struct.
The messages passed by the Spin Button widget to the applications' update function, in the updated version, will allow developers to update the current value of the Spin Button widget through the application state itself without calling an external spin_button::model::update(value) function. See the below screenshots as an example of this:
As can be seen, the Spin Button widget is created in the application view function, and like other widgets, the value is updated in the application update function. This keeps the application state in tact and keeps with the design of most of the other widgets in the library.
It can also be seen that the API signature is quite a bit more verbose. Although this can be cumbersome sometimes, I believe that it gives the developer more flexibility when the Spin Button widget is created, but doesn't have to be touched again after the creation itself and gives a one stop shop if something does need to be updated.
Also, I've added the flexibility of an orientation field that comes from a new Orientation enum that has two variants, Horizontal and Vertical. This field is designed as an Option type so that there could be a default behavior and a developer doesn't need to import the spin_button::Orientation enum if they just want a default Spin Button widget. This can be achieved by putting None into the orientation field on creation. This can be seen in the following screenshot from the example:
If a developer needs a standard spin button, I've made the API little shorter adding a spin_button_standard function that does not take the orientation parameter and just passes None to the SpinButton::new() function as the orientation parameter.
Another benefit in doing it this way is that the application state and messages can be created in a much cleaner manner, and only the initialization of the application state fields needs to happen. There is no Model struct initialization that needs to occur.
The widget does inherit the theme of the application, just like all other widgets do. Here is a screenshot of the example created with this PR and what the new Spin Button looks like in both orientations.
Here is a link to the working example of the proposed Spin Button widget in this PR.
Here is a link to the modified spin_button->mod.rs for review. It is heavily commented to mark out my thoughts and design choices as I was creating it.
As I realize that this is a breaking change, I am willing to help go through and resolve any issues that occur in the COSMIC default apps that would result if this change is accepted.
I’ve already identified a couple of places in cosmic-settings that would be affected by this and have forked cosmic-epoch to start working out how to repair what would be broken without affecting functionality.