Code review comment for lp:~widelands-dev/widelands/bug-1610507-disallowed-filename-characters

Revision history for this message
Notabilis (notabilis27) wrote :

Nice change, I like the tooltip! Works as intended and code is looking good.

Some remarks:
- Maybe add the tooltip to the OK button as well? At least for me that would be the more likely place where I would notice it.
- Is the smaller font for the list in the tooltip intentional?
- Maybe calculate the tooltip text only once and store it as static string in the file system class?

Not really related: I noticed that in the editor->make directory dialog the OK button is initially disabled, even though it should (?) be enabled. This is done intentionally by calling set_enabled(false) when creating the dialog, even though the directory name is valid and can be used. Is there a reason for disabling it?

review: Approve

« Back to merge proposal