Fixing the bug
First of all, I would like to address an issue you did not ask about. It is related to the situation where the user tries to copy without selecting any text. That is exactly the scenario I faced. This situation triggers an exception:
Exception in Tkinter callback
Traceback (most recent call last):
File "/usr/lib/python3.5/tkinter/__init__.py", line 1553, in __call__
return self.func(*args)
File "/home/begueradj/Desktop/popupkopy.py", line 26, in text_copy
text = self.textbox_text.get("sel.first", "sel.last")
File "/usr/lib/python3.5/tkinter/__init__.py", line 3095, in get
return self.tk.call(self._w, 'get', index1, index2)
_tkinter.TclError: text doesn't contain any characters tagged with "sel"
To fix this issue, simply check if the user selected a text and then allow him to perform the copy action. This is something you can express within the body of text_copy()
function as follows:
def text_copy(self, event=None):
if self.textbox_text.tag_ranges("sel"):
self.clipboard_clear()
text = self.textbox_text.get("sel.first", "sel.last")
self.clipboard_append(text)
Good decision?
But my code started to look a little messy so I decided to move all
the popup related methods and variables to its own class. And also
decided to write it that I can reuse my popup class for other projects
in the future that will require a popup menu.
A part of the software design, consists in identifying entities (objects/classes) of the problem you are trying to resolve. Thinking of the popup men as a class in itself is a right decision.
Discussing the implementation from the OOP perspective
- Inside the
Popup()
class, you can get rid of this declaration self.textbox_text = ''
as it is useless (never used, unlike what you may think) and a bit confusing. If you remove that variable, your program will still function correctly.
- I say
self.textbox_text = ''
is useless because what you actually are using in all Popup()
functions is rather the attribute you added to the instance of Popup()
inside MyTestApp()
when you coded: self.popup.textbox_text = self.data_textbox
. That is also why it is confusing.
- The remark 2 above means also that
MyTestApp()
and Popup()
classes are not compliant with the OOP philosophy because the first one must not be responsible of "expanding" the list of attributes of the second one. Objects have rather a state (Python attributes, in our case) and a behavior defined through an interface (not the interface in the strict OOP concepts) consisting of a set of methods and functions. The communication between objects occurs through their interfaces (functions/methods). And that is exactly why I am telling you this line: self.popup.textbox_text = self.data_textbox
does not comply with the OOP philosophy.
- How to fix the issue highlighted via the remark 3 above? Well, by just respecting the principles: a MyTestApp() instance must communicate with a Popup() instance (self.popup, in your case) through methods. I mean, you must find a way to communicate
self.data_textbox
to self.popup
object through a suitable method. This is too easy and clean as you will see in the improved version below.
Few other remarks
The previous section, especially the remark 4 above, responds fully to your questions. However, I would like to add few improvements suggestions to your code which I think you need to take in consideration as they are the widely respected standard. I will list some of them briefly:
- Avoid wildcard imports. This is not what I recommend, but what PEP 8 asks us to do:
Wildcard imports (from import *) should be avoided, as they
make it unclear which names are present in the namespace, confusing
both readers and many automated tools. There is one defensible use
case for a wildcard import, which is to republish an internal
interface as part of a public API (for example, overwriting a pure
Python implementation of an interface with the definitions from an
optional accelerator module and exactly which definitions will be
overwritten isn't known in advance).
When republishing names this way, the guidelines below regarding
public and internal interfaces still apply.
What does this mean in practice? It simply means you should remove these 2 lines: from tkinter import Tk, ttk, Frame, FALSE, Text, Button, \
Scrollbar, Entry, END, INSERT
and from tkinter import Frame, Entry, END, INSERT
and be content with import tkinter as tk
. This way you avoid useless imports you never used, but also you avoid repetition of the imports and, more importantly, in case you need to create a custom class/widget having the same name of one of the tkinter's classes, you will not face names clashes issues.
- Choose the right names: while this rule sounds easy to understand, not lot of us respect it even if it is too important for the development process. In my solution below, I renamed several of your variables.
- Last but not the least, follow Tkinter best practices.
Improved version
Taking in consideration all what is mentioned through the above sections, I came up with this improved version of your code (which surely can be subject to further improvements):
main.py:
from tkinter import ttk
import tkinter as tk
from popup import RightClickMenu
class MyTestApp(tk.Frame):
def __init__(self, parent):
self.master = parent
tk.Frame.__init__(self, self.master)
self.configure_gui()
self.create_widgets()
self.bind_right_click_menu_to_typing_area()
def configure_gui(self):
self.master.title('MY Test App')
def create_widgets(self):
self.create_text_area()
self.create_exit_button()
def create_text_area(self):
self.text_area = tk.Text(self.master, borderwidth=2, relief='sunken')
self.text_area.config(height=30, width=80)
self.text_area.grid(row=0, column=0, sticky="new")
def create_exit_button(self):
self.exit_btn = ttk.Button(self.master, text='Exit', command=self.exit_application)
self.exit_btn.grid(row=1, column=0, sticky='W', pady=15)
def bind_right_click_menu_to_typing_area(self):
self.popup = RightClickMenu(self.master, self.text_area)
self.text_area.bind("<Button-3>", self.popup.popup_text)
def exit_application(self):
self.master.destroy()
def main():
root = tk.Tk()
MyTestApp(root)
root.mainloop()
if __name__ == '__main__':
main()
popup.py:
import tkinter as tk
class RightClickMenu(tk.Frame):
def __init__(self, parent, text_area):
self.master = parent
tk.Frame.__init__(self, self.master)
self.text_area = text_area
self.create_widgets()
def create_widgets(self):
self.create_right_click_menu()
def create_right_click_menu(self):
self.right_click_menu = tk.Menu(self.master, tearoff=0, relief='sunken')
self.right_click_menu.add_command(label="Copy", command=self.copy_text)
self.right_click_menu.add_separator()
self.right_click_menu.add_command(label="Paste", command=self.paste_text)
self.right_click_menu.add_separator()
self.right_click_menu.add_command(label="Clear", command=self.clear_text)
def popup_text(self, event):
self.right_click_menu.post(event.x_root, event.y_root)
def copy_text(self):
if self.text_area.tag_ranges("sel"):
text = self.text_area.get("sel.first", "sel.last")
self.clipboard_append(text)
def paste_text(self):
self.text_area.insert(tk.INSERT, self.clipboard_get())
def clear_text(self):
self.text_area.delete(1.0, tk.END)
self.popup = Popup(parent)
. Also it seems thePopup()
class is in a separate module, which your actual code does not reflect within theMyTestApp()
class \$\endgroup\$from popup import Popup as p
Yes, the popup class is in a separate module called popup. I'm sorry if that was unclear, I should have worded this section better. so I decided to move all the popup related methods and variables to its own class. And also decided to write it that I can reuse my popup class for other projects in the future I'm not sure what you mean when you say it does not reflect in my code within in myMyTestApp()
\$\endgroup\$