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

Function 'addCheckBox' doesn't provide a text argument different from the title #161

Closed
jcolom64 opened this Issue May 9, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@jcolom64

jcolom64 commented May 9, 2017

Summary

Function 'addCheckBox' doesn't provide a text argument different from the title. The problem with this is that the title and text of the CheckBox widget is the same, therefore when creating multiple CheckBox widgets with the same text string an exception will be thrown due to duplicate key. Other widgets like addLabel provide arguments for both title and text, which allows to have multiple label with the same text string.

Exception in Tkinter callback

File "test.py", line 12, in
app.addCheckBox("CheckBox Text")
File "/usr/local/lib/python2.7/site-packages/appJar/appjar.py", line 3052, in addCheckBox
self.__verifyItem(self.n_cbs, title, True)
File "/usr/local/lib/python2.7/site-packages/appJar/appjar.py", line 2009, in __verifyItem
"Duplicate key: '" + item + "' already exists")
appJar.appjar.ItemLookupError: Duplicate key: 'CheckBox Text' already exists

How to Reproduce it

To show the issue, this code will create a Tabbed Frame with two Tabs that contains the same fields: a label and a checkbox. They idea is to be able to create clone tabs that will have similar fields inside each tab. In this particular case, notice that for the addCheckBox there is no way to specify the title and text in order to differentiate between the two CheckBox widgets.

from appJar import gui

app = gui("Sample Tap")
app.startTabbedFrame("TabFrame")

app.startTab("A")
app.addLabel("l1", "Label Text")
app.addCheckBox("CheckBox Text")
app.stopTab()
app.startTab("B")
app.addLabel("l2", "Label Text")
app.addCheckBox("CheckBox Text")
app.stopTab()

app.stopTabbedFrame()
app.go()

Solution

Add new argument text to the addCheckBox function similar to addLabel in order to specify a unique title separated from the text string.
tab

@jcolom64

This comment has been minimized.

Show comment
Hide comment
@jcolom64

jcolom64 May 9, 2017

I added an new parameter "text" and configured the default value for text. Looks like this solves the problem. Just replace the addCheckBox code with this:

def addCheckBox(self, title, text=None, row=None, column=0, colspan=0, 
                rowspan=0):
    self.__verifyItem(self.n_cbs, title, True)
    var = IntVar(self.topLevel)
    cb = Checkbutton(self.__getContainer())
    if text is not None:
        cb.config(text=text)
        cb.DEFAULT_TEXT = text
    else:
        cb.DEFAULT_TEXT = ""
    cb.config(
        variable=var,
        font=self.cbFont,
        background=self.__getContainerBg(),
        activebackground=self.__getContainerBg())
    cb.config(anchor=W)
    cb.bind("<Button-1>", self.__grabFocus)
    self.n_cbs[title] = cb
    self.n_boxVars[title] = var
    self.__positionWidget(cb, row, column, colspan, rowspan, EW)

jcolom64 commented May 9, 2017

I added an new parameter "text" and configured the default value for text. Looks like this solves the problem. Just replace the addCheckBox code with this:

def addCheckBox(self, title, text=None, row=None, column=0, colspan=0, 
                rowspan=0):
    self.__verifyItem(self.n_cbs, title, True)
    var = IntVar(self.topLevel)
    cb = Checkbutton(self.__getContainer())
    if text is not None:
        cb.config(text=text)
        cb.DEFAULT_TEXT = text
    else:
        cb.DEFAULT_TEXT = ""
    cb.config(
        variable=var,
        font=self.cbFont,
        background=self.__getContainerBg(),
        activebackground=self.__getContainerBg())
    cb.config(anchor=W)
    cb.bind("<Button-1>", self.__grabFocus)
    self.n_cbs[title] = cb
    self.n_boxVars[title] = var
    self.__positionWidget(cb, row, column, colspan, rowspan, EW)
@jarvisteach

This comment has been minimized.

Show comment
Hide comment
@jarvisteach

jarvisteach May 25, 2017

Owner

Thank you for this. Unfortunately, it will break backwards compatibility - anyone who upgrades and is already using checkboxes, and has specified a row/column/etc will have issues...

But, you're right, this is an issue. We did a similar thing with buttons back in the day - introducing addNamedButton(), I think we'll need the same for CheckBoxes - 'addNamedCheckBox()'

I'll add it to the list for the next release.

Owner

jarvisteach commented May 25, 2017

Thank you for this. Unfortunately, it will break backwards compatibility - anyone who upgrades and is already using checkboxes, and has specified a row/column/etc will have issues...

But, you're right, this is an issue. We did a similar thing with buttons back in the day - introducing addNamedButton(), I think we'll need the same for CheckBoxes - 'addNamedCheckBox()'

I'll add it to the list for the next release.

@jarvisteach jarvisteach self-assigned this May 25, 2017

@jarvisteach jarvisteach added this to the 0.07 milestone May 29, 2017

jarvisteach added a commit that referenced this issue May 29, 2017

NamedCheckBox (#161)
New function: `addNamedCheckBox()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment