search

ParthJadhav / Tkinter-Designer

An easy and fast way to create a Python GUI 🐍
BSD 3-Clause "New" or "Revised" License
8.87k stars 817 forks source link

Fixed bugs + added Ellipse and Line #212

Closed avivfaraj closed 1 year ago

avivfaraj commented 2 years ago

Fixed some bugs: 1) All shapes appear in black - Changed the if statements in frame.py from element_name to element_type. Also, modified color method in vector_elements.py to take into account strokes (outline color) 2) "Rectangle cannot be parse" - Change if statement from "Rectangle" to "VECTOR".

Additions: 1) Line shape - added Line + fixed its color + width 2) Ellipse - Added code for Ellipse + fixed color + outline color

ParthJadhav commented 2 years ago

Hey @avivfaraj Thanks for the PR. I'll be reviewing the changes soon and will get back to you. Sorry for the delay.

ParthJadhav commented 2 years ago

Hey @avivfaraj , I found that you changed the checks from Name to Type. Sadly this won't work. Because Figma API doesn't return the element type as we want. It doesn't know which element is what. It just knows that this is a Rectangle or This is a Square. That's why we need naming convention for the conversion.

ParthJadhav commented 2 years ago

Can we only include the Stroke & Ellipse feature from this PR ? I already merged a Line feature PR.

avivfaraj commented 2 years ago

Hey @avivfaraj , I found that you changed the checks from Name to Type. Sadly this won't work. Because Figma API doesn't return the element type as we want. It doesn't know which element is what. It just knows that this is a Rectangle or This is a Square. That's why we need naming convention for the conversion.

I am not sure I understand what your are saying. Checking the name doesn't make sense, since you can name each element differently, for instance, "rectangle 360 RC". In this case, your if statement will always fail, which results in a black rectangle because the algorithm didn't find that shape. On the other hand, each element in the API response has a "type" field that indicate which element it is, so it is much better to use it for defining your elements. Attached an image to clarify my point.Screen Shot 2022-06-12 at 10 52 38 AM

As you can see in the image, the name is marked in a white rectangle, and type in red rectangles. Using your name method, the conversion will always fail because the name is not exactly "RECTANGLE" or "LINE" or "ELLIPSE" because it can be anything I want to (it is a name), whereas type is consistent. There are other type field out there in the document, but you can ignore them, recall that it is json which mean nested dictionaries.

avivfaraj commented 2 years ago

Can we only include the Stroke & Ellipse feature from this PR ? I already merged a Line feature PR.

Yeah