feat: Enable editing width of existing strokes#1752
Conversation
Doublonmousse
left a comment
Doublonmousse
left a comment
There was a problem hiding this comment.
I'm not 100 % sure as far as UI goes for this (is this the best place for it/should we place this here etc...) so I'm not guaranteeing this will be the solution we keep for editing the stroke width.
It's best to wait for another review for that aspect (not by me at least)
I do have some other remarks though
Also, I'd like to save the size of the selector buttons between application restarts, like in the other width selectors, but I haven't been able to figure out how to do this yet.
See https://github.com/flxzt/rnote/blob/main/crates/rnote-ui/src/appwindow/appsettings.rs#L309-L319 for the setting permanence (note this also means changing the https://github.com/flxzt/rnote/blob/main/crates/rnote-ui/data/app.gschema.xml.in as well.
Not sure whether keeping the same settings as the brushstroke makes sense or not (if they are separate then they move independently, if not changing it in one place changes it in another so ... maybe this wouldn't be so clear and accidentally changing one of your preset by resizing the stroke width could happen if one of the setter is selected).
| #[template_child] | ||
| pub(crate) stroke_width_picker: TemplateChild<RnStrokeWidthPicker>, | ||
|
|
||
| pub(crate) allow_stroke_width_changes: Cell<bool>, // Used to stop the stroke width changing immediately when the popup is opened |
There was a problem hiding this comment.
I want to do something different for this.
The Cell means we cannot derive the default, is that correct ?
There was a problem hiding this comment.
That is correct.
Another way to do this withough the cell is to do the following
* Call `get_selection_stroke_width` * Compare the stroke width value to the `stroke_width` * If they are not identical (to some precision) then change the stroke width with `change_selection_stroke_width`
This seems like a much simpler method, thanks for the suggestion.
|
|
||
| imp.allow_stroke_width_changes.set(false); // Stop the new picker width from applying immediately | ||
| imp.stroke_width_picker | ||
| .set_stroke_width(stroke_width + 0.001); // A small value is added to allow the user to apply a width equal to 'stroke width' |
There was a problem hiding this comment.
I'm not getting why the +0,001 is needed
| let stroke_width = canvas | ||
| .engine_ref() | ||
| .get_selection_stroke_width() | ||
| .unwrap_or(2.0); // Default to width of 2 if no valid strokes selected |
There was a problem hiding this comment.
Another way to do this would be to have the button be sensitive depending on the selection state. So no way to actually open the popover if there is nothing valid to change the stroke width on.
This is a little more involved as far as code goes though
| // Return if the popup has just been opened | ||
| if !imp.allow_stroke_width_changes.get() { | ||
| imp.allow_stroke_width_changes.set(true); | ||
| return; | ||
| } | ||
|
|
||
| // Change the width of the selected strokes | ||
| let widget_flags = canvas | ||
| .engine_mut() | ||
| .change_selection_stroke_width(stroke_width); |
There was a problem hiding this comment.
Another way to do this withough the cell is to do the following
- Call
get_selection_stroke_width - Compare the stroke width value to the
stroke_width - If they are not identical (to some precision) then change the stroke width with
change_selection_stroke_width
I feel like from a UX perspective, having a unified size picker in the top bar next to the colors would be a very clean solution:
PS: Just putting this up for discussion, not saying that it should be done this way. |
When the selected strokes have different widths, the width picker displays one of them in the input box. If the user wants to apply this width to all selected strokes, they would need to change the value in the input box to something different, then change it back. The +0.001 helps with this, as it lets the user apply the width to all selected strokes by selecting the input box and pressing 'enter', as the rounding of the value is detected as a change. I'm not particularly happy with this solution though, and on further reflection it doesn't resolve the issue for pen/touch input methods. I'm thinking of creating a custom width picker widget instead, with the following behavior: If all selected strokes have the same width:
If selected strokes have different widths:
It will likely be a week or so before I have time to work on this, but does it sound okay as a solution? |
3 participants
This merge request adds a popup menu to the selection sidebar, which allows the width of the selected strokes to be edited. The widths of selected brush and shape strokes are changed, while other stroke types are unaffected.
Resolves #1039.
Partially resolves #1626 , by allowing the width to be manually changed back after an object is scaled.
Demo:
demo.mp4
Todo:
Additionally, there are 2 problems I've not yet been able to resolve:
The 3 width selector buttons are deselected when the popup is closed to allow them to be clicked again next time it is opened. The issue is that the deselect animation only starts when the popup is next opened, and I don't know how to stop this animation from playing:
bug-demo.mp4
Also, I'd like to save the size of the selector buttons between application restarts, like in the other width selectors, but I haven't been able to figure out how to do this yet.
Any advice on these issues would be greatly appreciated. I would also like to confirm whether the implementation so far looks suitable, or if there are any changes needed.
Thanks for making such a great app!