Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] DrawingView.ClearOnFinish seems to have no effect #1453

Closed
jfversluis opened this issue Jul 1, 2021 · 6 comments
Closed

[Bug] DrawingView.ClearOnFinish seems to have no effect #1453

jfversluis opened this issue Jul 1, 2021 · 6 comments
Labels
a/DrawingView bug Something isn't working. Breaky break.

Comments

@jfversluis
Copy link
Member

Description

Maybe I'm misunderstanding what this does. I expect that this property would leave my current drawing in place when I start to draw a second line. However, this is not the case. Khttps://github.com/xamarin/XamarinCommunityToolkit/blob/develop/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/DrawingView/Renderer/DrawingViewRenderer.ios.cs#L79 it is correctly checked and the collection is not cleared, however here the collection is still cleared.

Only tested on iOS, but looking at the Android renderer same thing happens there, so will be good to check all platforms.

Steps to Reproduce

  1. Implement a DrawingView like below
<xct:DrawingView ClearOnFinish="False" LineColor="Red" BackgroundColor="LightGray" HeightRequest="200" Points="{Binding DrawPoints}" DrawingCompletedCommand="{Binding DrawCompleted}" />
  1. Draw on the canvas and stop drawing by raising your finger/pointer
  2. Draw a second line and observe that the first line is erased

Expected Behavior

Both lines are shown on the canvas

Actual Behavior

Only one line at a time is shown on the canvas

Basic Information

  • Version with issue: Current nightly
  • Last known good version: N/A
  • IDE: VSMac
  • Platform Target Frameworks:
    • iOS: 14.5
    • Android: Not tested on Android, but looking at the code it also happens here

Workaround

None that I know of right now

Reproduction imagery

Reproduction Link

@jfversluis jfversluis added bug Something isn't working. Breaky break. a/DrawingView labels Jul 1, 2021
@jfversluis
Copy link
Member Author

@VladislavAntonyuk FYI

@VladislavAntonyuk
Copy link
Contributor

VladislavAntonyuk commented Jul 1, 2021

@jfversluis It is expected behavior. We should clear the screen when the customer starts drawing independently from the ClearOnFinish.
ClearOnFinish="True" means that the image clears on completing the drawing.

We do not allow multiple lines, because of Points collection. it should be Collection of Collection of Points to support multiple lines.
I am not sure would it be a breaking change in the future or we will be able to add a Strokes property, but for now we support only a single line.

I described some properties here: https://vladislavantonyuk.azurewebsites.net/articles/Drawing-View-in-Xamarin-Community-toolkit

@jfversluis
Copy link
Member Author

Interesting, I see how it's supposed to work now, thanks for clearing that up.

I think it would be nice to also have the ability to draw multiple lines independently. I don't fully understand why that should be a problem? The points can be duplicate or overlapping or anything right? But maybe I'm not fully understanding how this works. We should follow up in a new issue probably

@VladislavAntonyuk
Copy link
Contributor

we draw the line as an array of points (we draw the line between two points). so when we set the points, we will draw the line between the last point of "Line1" and the first point of "Line2".
To support multiple lines we need a new property Strokes (Array of Array of Points). And furthermore, I believe in that case we need to remove Points property. which is a breaking change.
How much time do we have before 1.2 release? I can try to implement Strokes property.

@jfversluis
Copy link
Member Author

Fixed in #1459

@jfversluis
Copy link
Member Author

jfversluis commented Jul 2, 2021

we draw the line as an array of points (we draw the line between two points). so when we set the points, we will draw the line between the last point of "Line1" and the first point of "Line2".
To support multiple lines we need a new property Strokes (Array of Array of Points). And furthermore, I believe in that case we need to remove Points property. which is a breaking change.
How much time do we have before 1.2 release? I can try to implement Strokes property.

I'm releasing it NOW! :P So don't worry about it. We'll make it something for the next release

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/DrawingView bug Something isn't working. Breaky break.
Projects
None yet
Development

No branches or pull requests

2 participants