-
-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added canvas context menu and duplicate layer command #162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Insert (1) at the end of the duplicated layer name, add undo support. I don't think you need to write any undo process, you can reuse what already exists
Codecov Report
@@ Coverage Diff @@
## master #162 +/- ##
==========================================
+ Coverage 52.34% 52.49% +0.15%
==========================================
Files 236 237 +1
Lines 8953 9030 +77
Branches 867 874 +7
==========================================
+ Hits 4686 4740 +54
- Misses 4041 4061 +20
- Partials 226 229 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is clean, everything works, I was unable to break it, you've reused the code well and even wrote a test for suffixes, perfect. The only thing I need to ask you is to write a test for the Duplicate layer. Nothing fancy, just if it's added to Layers and is active, you don't need to compare contents or test undo.
private int? GetHighestSuffix(Layer except, string layerName, Regex regex) | ||
{ | ||
int? highestValue = null; | ||
|
||
foreach (Layer otherLayer in Layers) | ||
{ | ||
if (otherLayer == except) | ||
{ | ||
continue; | ||
} | ||
|
||
Match otherMatch = regex.Match(otherLayer.Name.Reverse()); | ||
|
||
if (otherMatch.Groups[2].Value == layerName) | ||
{ | ||
bool sucess = int.TryParse(otherMatch.Groups[1].Value.Reverse(), out int number); | ||
|
||
if (sucess) | ||
{ | ||
if (highestValue == null || highestValue < number) | ||
{ | ||
highestValue = number; | ||
} | ||
} | ||
else | ||
{ | ||
if (highestValue == null) | ||
{ | ||
highestValue = 0; | ||
} | ||
} | ||
} | ||
} | ||
|
||
return highestValue; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit too long function, split it a little, please :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
Added a canvas context menu, containing (De)select all, Copy, Cut, Paste, Bake Pizza and added a duplicate layer command to the layer list