search

lawsie / guizero

A Python 3 library to allow learners to quickly and easily create GUIs.
https://lawsie.github.io/guizero
BSD 3-Clause "New" or "Revised" License
406 stars 81 forks source link

Add missing text properties to text derived widgets and containers (weight, slant, underline & overstrike) #506

Open Blindstars opened 6 months ago

Blindstars commented 6 months ago

Related to my post in the Discussions:

This PR adds "weight", "slant", "underline" and "overstrike to most text derived widgets including containers and drawings. I included tests for all of them, updated docs and "Text" example.

"TitleBox" inheriting from "ContainerTextWidget" is not able to have text with overstrike. Since it is inheriting from "Container" properties can cascade or be inherited. It is doing underline instead. I don't like the way I handled the tests for this edge case. We could also add a "warning" somewhere in "TitleBox" but the inheritance is way down...

First contribution ever and open to feedback ! I am not a software engineer, but a mechanical one... Don't take this as an excuse, I want to learn the ropes.

martinohanlon commented 6 months ago

Thank you. It's great to have contributions from new people.

Often they don't include tests or documentation so thanks for being so thorough.

I have only had a skim through the files changed and not tested it.

One thing came to mind when looking through the docs.

Are the only permissable values for weight "bold" and "normal"? If so could the API be simplified by having a Boolean method of bold?

Is the same also true for slant with "italic" and "normal".

I don't know if there are other weights or slants.

Blindstars commented 6 months ago

Thanks !

From what I saw in the Tk/Tcl docs, there are only "normal/bold" for weight and "roman/italic" for slant. I pretty much followed their API looking at tkinter.font.Font and the docs:

https://www.tcl.tk/man/tcl8.6/TkCmd/font.htm#M24

However, I do agree that we could simplify the API for guizero by making them all boolean parameters.

Happy to update the code, tests and docs reflecting this.

lawsie commented 5 months ago

Thanks so much, lovely to have a PR! I agree with Martin that it would be simpler if there are only two values to have it as a Boolean. I also don't like the terms 'weight' and 'slant' - could this just be bold and italic? (I understand these have come from tkinter and not from you @Blindstars !) I would imagine many more beginners would be familiar with bold and italic from e.g. a word processor and the aim is to keep things as simple and straightforward as possible.

Blindstars commented 5 months ago

Makes sense to me !

I'll update the PR this weekend.

Thanks for the feedback!

From: Laura Sach @.> Sent: Friday, June 7, 2024 4:30:09 a.m. To: lawsie/guizero @.> Cc: Blindstars @.>; Mention @.> Subject: Re: [lawsie/guizero] Add missing text properties to text derived widgets and containers (weight, slant, underline & overstrike) (PR #506)

Thanks so much, lovely to have a PR! I agree with Martin that it would be simpler if there are only two values to have it as a Boolean. I also don't like the terms 'weight' and 'slant' - could this just be bold and italic? (I understand these have come from tkinter and not from you @Blindstarshttps://github.com/Blindstars !) I would imagine many more beginners would be familiar with bold and italic from e.g. a word processor and the aim is to keep things as simple and straightforward as possible.

— Reply to this email directly, view it on GitHubhttps://github.com/lawsie/guizero/pull/506#issuecomment-2154352974, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AW737TH5ODHDOYJUZZCYTPDZGFVQ3AVCNFSM6AAAAABIL5YEOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJUGM2TEOJXGQ. You are receiving this because you were mentioned.Message ID: @.***>

Blindstars commented 5 months ago

Updated according to the discussion.

martinohanlon commented 5 months ago

This looks ok to me. I would suggest we merge before the next release.