Skip to content

Styling dialogs #5333

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

Conversation

Alpha-Centauri-00
Copy link

i made some new styling for Dialogs library..
i was trying to get the same style as robotframework.org .. and i think it needs more marging for some inputs but that should not be a big thing.. i "hard coded" colors in init to change it later if nessessery ..

i did not change any functionality of the keywords.. only the colors and buttons ... i test it on windows and linux (debian) only. i wish its gonna work with mac..

image

image

image

image

@franzhaas
Copy link
Contributor

while not related on a technical level, it still touches the same code base and requires the same manual tests.:

#5335

@Alpha-Centauri-00
Copy link
Author

Alpha-Centauri-00 commented Feb 19, 2025

the mac issue is solved now and its working after my last commit. but still looks very weird on mac as you can see in the photo below.

we'll see how we can make a work around with Andre on mac ...

image

@Alpha-Centauri-00
Copy link
Author

Due to compatibility issues with macOS, the canvas element is not functioning as expected for styling buttons. To address this, alternative technologies can be employed to achieve the desired styling. If left unchanged, the default appearance of the button remains decent, and also the color modification to the button on macOS is not possible, unless using ttk or customtkinter libraries.

for now i think mac is okay

@Alpha-Centauri-00
Copy link
Author

Styling apply now on windows and linux. mac is going to show the old default styling. a big thanks to @AndreMochinin for helping testing this on mac.

@pekkaklarck
Copy link
Member

pekkaklarck commented Mar 22, 2025

I finally had time to look at this properly. I tested the changes locally and also looked the code quickly. Below is an example showing the same dialog first with the current styles and then with the styles from this PR.

Kuvakaappaus 2025-03-22 11-11-09

As I've written earlier, in general I like the idea of enhancing the styles and using the official Robot Framework colors. In my opinion this gets a bit too far, though, and there are also some technical problems (bugs) as well as . I'll clarify my concerns below.

Style issues

  1. I don't understand why the normal buttons for minimizing, resizing and closing dialogs are removed. Are the some concrete benefits from that? I can see them being useful in some cases.
  2. Dialog title not being centered makes it harder to separate it from the dialog text.
  3. Input widget not using all the horizontal space is odd.
  4. Buttons are unnecessary small.
  5. Overall I'd prefer the dialogs to look more like platform native dialogs. This is obviously subjective and others can disagree.

Bugs

  1. Dialogs aren't closed properly at least on Linux. The dialog itself goes away, but the entry in the panel where open applications are shown stays until the execution ends. The panel gets very crowded if you use multiple dialogs.
  2. The aforementioned panel entry only shows tk as the text when it used to show Robot Framework.
  3. The dialog disappears when I move it down or right from the original position. It reappears when I release the mouse, vanishing while dragging is annoying. The dialog can also flash.
  4. Dialogs not working the same on all platforms isn't acceptable. I don't like it in general, but the bigger problems is that further changes need to take different platforms into account.
  5. The underlining of the button shortcut key has been removed (see O and C in the example).

Code issues

The changes look pretty complicated and that's likely the reason for some of the above bugs. Even if the bugs were fixed, complicated code is a huge problem from maintainability point of view.

@pekkaklarck
Copy link
Member

Due to the problems explained my previous comment, I don't think this big changes make sense. Even if the bugs and style issues would be fixed, I don't consider the benefits big enough compared to the maintenance issues caused by the added complex code. The code not working on OSX also makes me worried that further changes to tkinter can break it also on other platforms.

I'd still be happy with the dialogs being enhanced a bit, but changes should have the following characteristics:

  • Must work on all our supported platforms.
  • No platform specific conditional logic unless the benefits are really big.
  • No complex logic unless benefits are extremely big. This avoids the code being broken if tkinter changes and eases maintenance. As an example, I don't consider rounded corners important enough to warrant much extra code.
  • Bugs mentioned in the previous comment fixed.

Now that I think about this, very simple changes like using a custom color in the dialog title background could be enough. If you want fancier dialogs, you could also create an external project for that. It could then have additional dependencies, didn't need to support all OSes, and so on.

@pekkaklarck
Copy link
Member

I looked at changing the title background color, but it seems that it's not supported by tkinter directly. A workaround, that also this PR uses, is replacing the default title bar with a custom title bar. Unfortunately removing the title bar with self.overrideredirect(True) also makes it impossible to move the dialog. This PR seems to workaround that with custom code that supports moving, but that adds extra complexity. Apparently the code also has issues, because there are problems moving the dialog. I don't think this kind of changes are worth the complexity in the core.

Changing the background colors otherwise is simple, though. For example, the following version was created by only changing background of widgets we already used (and also added a little padding that's probably a good idea regardless of styles). The diff of the changes is below the example.

dialog

--- a/src/robot/libraries/dialogs_py.py
+++ b/src/robot/libraries/dialogs_py.py
@@ -48,6 +48,7 @@ class TkDialog(Toplevel):
         self.withdraw()    # Remove from display until finalized.
         self.title('Robot Framework')
         self.protocol("WM_DELETE_WINDOW", self._close)
+        self.configure(background='#00c0b5', padx=5, pady=5)
         self.bind("<Escape>", self._close)
         if self.left_button == TkDialog.left_button:
             self.bind("<Return>", self._left_button_clicked)
@@ -69,9 +70,10 @@ class TkDialog(Toplevel):
             self.widget.focus_set()
 
     def _create_body(self, message, value, **config) -> Union[Entry, Listbox, None]:
-        frame = Frame(self)
+        frame = Frame(self, background='#00c0b5')
         max_width = self.winfo_screenwidth() // 2
-        label = Label(frame, text=message, anchor=W, justify=LEFT, wraplength=max_width)
+        label = Label(frame, text=message, anchor=W, justify=LEFT, wraplength=max_width,
+                      background='#00c0b5', pady=10)
         label.pack(fill=BOTH)
         widget = self._create_widget(frame, value, **config)
         if widget:
@@ -83,7 +85,7 @@ class TkDialog(Toplevel):
         return None
 
     def _create_buttons(self):
-        frame = Frame(self)
+        frame = Frame(self, background='#00c0b5')
         self._create_button(frame, self.left_button, self._left_button_clicked)
         self._create_button(frame, self.right_button, self._right_button_clicked)
         frame.pack()

The main motivation of the above isn't to show certain styles, but to show what kind of changes would be acceptable. If the above would be used as a base, the background color should be moved to class variable and styling buttons should be considered.

Another thing to consider is using the tkinter.ttk module that apparently has better support for styling. I don't know would it give that much benefits in our case, though, and it would also need to be tested on different platforms.

@pekkaklarck
Copy link
Member

The complexity and the problems caused by it aren't worth the benefits and I'll close this PR. Simpler enhancements are still possible. I commented the issue #5334 about things that I think can be changed.

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.

3 participants