FashionFreedom / Seamly2D

Open source patternmaking software to democratize fashion.
https://seamly.io
GNU General Public License v3.0
617 stars 111 forks source link

Feature: Add a "from the end" option to the "Point on spline" tool to enter the distance from the end of the spline to the point. #1132

Closed Onetchou closed 4 months ago

Onetchou commented 5 months ago

Add a "from the end" checkbox to the "Point on spline" tool to enter the distance from the end of the spline to the point instead of from the beginning. image

DSCaskey commented 5 months ago

Working on it. While I'm at it I'm going to also implement the feature with Point - On Curve and Point - On Arc as it's the same behavior depending on the direction of the curve or arc. It also makes sense to add them all to a new schema at the same time.

DSCaskey commented 5 months ago

I'm thinking of using mutually exclusive radio buttons instead: It makes it clearer what the 2 options are.

image

Onetchou commented 5 months ago

I'm thinking of using mutually exclusive radio buttons instead: It makes it clearer what the 2 options are.

image

Thats's far better than the checkbox I suggested indeed! 😊

Thank you!

DSCaskey commented 5 months ago

@Onetchou...

What do you think about using a Combobox for the 2 options instead? As I'm adding the methods to the tools, I realized I need to add a tool direction get & set method in the base class VToolCut that the 3 tools share. Which BTW is another reason we sort of have to add the feature to all 3 tools as they share the same base class. These would be used in the Properties Editor to access the direction data - which is not needed with the popup tool dialogs. If I was to use radio buttons I'd have to create a new Property plugin for the Property Editor (browser)... where if I use a combobox we already have a Property plugin for that. It's not without precedent as there are other cases where the same idea is used... like selecting which "take" to use for the Point - Intersect Circle & Tangent:

image

It would have been SO much easier if RT had simply used a common widget in the tool dialogs and Property Editor dock... the whole third party PropertyBrowser lib would have been unnecessary.

Oh... and I figured this text for the 2 options makes more sense from a code naming standpoint of:

Direction:

Where the options are enum'd Direction::Forward , Backward

Onetchou commented 5 months ago

@Onetchou...

What do you think about using a Combobox for the 2 options instead? As I'm adding the methods to the tools, I realized I need to add a tool direction get & set method in the base class VToolCut that the 3 tools share. Which BTW is another reason we sort of have to add the feature to all 3 tools as they share the same base class. These would be used in the Properties Editor to access the direction data - which is not needed with the popup tool dialogs. If I was to use radio buttons I'd have to create a new Property plugin for the Property Editor (browser)... where if I use a combobox we already have a Property plugin for that. It's not without precedent as there are other cases where the same idea is used... like selecting which "take" to use for the Point - Intersect Circle & Tangent:

image

It would have been SO much easier if RT had simply used a common widget in the tool dialogs and Property Editor dock... the whole third party PropertyBrowser lib would have been unnecessary.

Oh... and I figured this text for the 2 options makes more sense from a code naming standpoint of:

Direction:

  • Forward (from the start point)
  • Backward (from the end point)

Where the options are enum'd Direction::Forward , Backward

Even if radio buttons seem more suitable for only 2 options, I agree that it's not worth the challenge of creating a new property plugin 😉

DSCaskey commented 5 months ago

Even though I like radio buttons when there's a limited number of buttons - as you can see all the options without scrolling a dropdown... they do have more overhead in that you have to do conversions between the radiobutton id's and strings through switch's, whereas you can directly access a string as the combobox's data... unless you want dom attributes like direction = "0" or direction = "1", which is not as clear as direction = "forward" or direction = "backward".

On Fri, Jun 28, 2024, 3:57 PM Onetchou @.***> wrote:

@Onetchou https://github.com/Onetchou...

What do you think about using a Combobox for the 2 options instead? As I'm adding the methods to the tools, I realized I need to add a tool direction get & set method in the base class VToolCut that the 3 tools share. Which BTW is another reason we sort of have to add the feature to all 3 tools as they share the same base class. These would be used in the Properties Editor to access the direction data - which is not needed with the popup tool dialogs. If I was to use radio buttons I'd have to create a new Property plugin for the Property Editor (browser)... where if I use a combobox we already have a Property plugin for that. It's not without precedent as there are other cases where the same idea is used... like selecting which "take" to use for the Point - Intersect Circle & Tangent:

[image: image] https://private-user-images.githubusercontent.com/31944718/344020234-d6ae7acd-f923-484e-bd20-61f1921bffd8.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTk2MDQ2NzIsIm5iZiI6MTcxOTYwNDM3MiwicGF0aCI6Ii8zMTk0NDcxOC8zNDQwMjAyMzQtZDZhZTdhY2QtZjkyMy00ODRlLWJkMjAtNjFmMTkyMWJmZmQ4LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA2MjglMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNjI4VDE5NTI1MlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTA4NWJkMmZiN2E4YmQ0YjJkZTZkZTA2MmJkZjZmZGYzZTM2MTAxNjI3YzJkNDQyZDZmMDA5Mjg1YTBkZTdiNWUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.2Fbi1riaO6myrcrxmEHlG82FqdoGeSXA6ziyL0oCNnc

It would have been SO much easier if RT had simply used a common widget in the tool dialogs and Property Editor dock... the whole third party PropertyBrowser lib would have been unnecessary.

Oh... and I figured this text for the 2 options makes more sense from a code naming standpoint of:

Direction:

  • Forward (from the start point)
  • Backward (from the end point)

Where the options are enum'd Direction::Forward , Backward

Even if radio buttons seem more suitable for only 2 options, I agree that it's not worth the challenge of creating a new property plugin 😉

— Reply to this email directly, view it on GitHub https://github.com/FashionFreedom/Seamly2D/issues/1132#issuecomment-2197544287, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHTXADSFOZDOK2SLW4BQJ2LZJW53PAVCNFSM6AAAAABJ4QFIVCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJXGU2DIMRYG4 . You are receiving this because you commented.Message ID: @.***>