oxyplot icon indicating copy to clipboard operation
oxyplot copied to clipboard

Add PathAnnotation.TextBackground properties (#1900)

Open VisualMelon opened this issue 3 years ago • 1 comments

Fixes #1900 by adding properties to PathAnnotation that match the functionality of TextAnnotation. Decided to not try to put everything in TextualAnnotation because there would be naming issues, as the same names are used in PathAnnotation and TextAnnotation already for different purposes.

Checklist

  • [x] I have included examples or tests
  • [x] I have updated the change log
  • [x] I am listed in the CONTRIBUTORS file
  • [x] I have cleaned up the commit history (use rebase and squash)

Changes proposed in this pull request:

  • Add four new properties to PathAnnotation:
    • TextBackground
    • TextBackgroundStroke
    • TextBackgroundStrokeThickness
    • TextBackgroundPadding

@colejonson66 suggested Border for the stroke properties as an alternative to Background, which may be a better name.

@oxyplot/admins

VisualMelon avatar Aug 13 '22 16:08 VisualMelon

ShapeAnnotation would be another candidate for adding such properties if we can settle on names.

VisualMelon avatar Aug 13 '22 16:08 VisualMelon

@oxyplot/admins does this API look OK? Not wild about ChangeOpacity now that I look at it again, but can remove that and keep the other changes otherwise.

Any preference between Border/Background for naming per ermakrs from @colejonson66 ?

VisualMelon avatar May 14 '23 12:05 VisualMelon

Border sounds more adequate at first, however the property TextBackground doesn't make sense when renamed to TextBorder. I guess we'd have to do TextBorderBackground then. TextBorderStroke, TextBorderStrokeThickness and TextBorderPadding would be fine however.

Currently I would tend to leaving it at Background, but could be convinced otherwise.

ChangeOpacity is fine in my opinion, ChangeAlpha could also work.

Jonarw avatar Jul 11 '23 17:07 Jonarw

Looking at this quickly now, I'm tempted again by TextBorderStroke and TextBorderThickness, leaving everything else as is. If that's agreeable, then I'll make that change and I think we should get this merged.

VisualMelon avatar Jul 22 '23 08:07 VisualMelon

Sounds good to me!

Jonarw avatar Jul 22 '23 09:07 Jonarw

Then I don't like TextBackgroundPadding... I think TextBorderBackground is OK, so I've changed to Border everything (rather than TextBackground and rebased. I don't know if you're convinced by the new naming, however.

VisualMelon avatar Jul 22 '23 16:07 VisualMelon