Skip to content
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 new padding widget #448

Closed
wants to merge 2 commits into from
Closed

Added new padding widget #448

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 23, 2022

Size of padding can be controlled in pt, with size=XX.

@martinohanlon
Copy link
Collaborator

Is there any more information about this PR? There is no documentation to support it.

It looks like this is a widget which can be used to add padding to a layout. So not dissimilar to a Box but you can size it as characters rather than pixels? Is that right?

My first thought is that there are lots of others ways of implementing padding. Modify Box, add padding properties to existing widgets.

If implementing a Padding widget how could it support other sizes? I'll put some comments against the commit.

def __init__(
self,
master,
size=None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a benefit of having a padding with a None size?

# ----------------------------------
# The text value
@property
def value(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should you be able to set the value of some Padding?

# METHODS
# -------------------------------------------

# Clear text (set to empty string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear and append methods for a Padding seem superfluous.

from . import utilities as utils
from .base import TextWidget

class Padding(TextWidget):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By inheriting from TextWidget a Padding will inherit all the properties of a widget with text. Need to check if a padding should support the attributes of a TextWidget.

grid=None,
align=None,
visible=True,
enabled=None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a Padding be enabled / disabled?

@martinohanlon
Copy link
Collaborator

@Captain-Toad please dont take my comments as criticism :) adding a new widget to guizero can have a significant impact and should be done after consideration of other options and the potential issues this could cause. Thank you for your contribution, is useful to continue the conversation.

@martinohanlon
Copy link
Collaborator

Referencing issue #361

@ghost
Copy link
Author

ghost commented Apr 23, 2022

Sorry about that. I could not see any other way to create padding other than the Text widget, and to automate it I created this. The file is a copy of the Text widget, but with things like font and colour removed.

Repository owner closed this by deleting the head repository Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants