GNOME Bugzilla – Bug 649070
Use RadioButtons for changing font in the Preferences dialog
Last modified: 2020-11-24 09:57:28 UTC
Created attachment 186962 [details] [review] Patch that fixes the issue Currently in the Preferences dialog, in the Font & Colors tab, underneath the heading 'Font', a CheckButton is used to describe two mutually exclusive events. This can be quite confusing/not immediately obvious, as it is not clear that the top CheckButton controls the sensitivity of the FontButton below it. Attached is a patch that fixes this problem, by adding two RadioButtons, one for each event, that helps to distinguish the fact that these are two mutually exclusive events. Apart from this, everything else acts in the same way as before. Attached you can also see a screenshot, with a before and after. Can somebody review this patch, so that it can be merged into trunk? Thanks :)
Created attachment 186963 [details] Before and After Screenshot
(I should also mention in the patch, I changed all the horizontal alignments of the CheckButtons/RadioButtons already in the dialog to be 0.0, this just assures that they are aligned to the left.)
Review of attachment 186962 [details] [review]: Hi, thanks for the time to report this and make a patch. I see the argument about "mutual exclusion", on the other hand I always find radio buttons with just two items a bit odd and I think the current ui makes a better use of space since it does not have two "white circles". That said I am open to this change if the UI people think it is a good idea. If we merge it however we probably want to use different strings for the labels: I do not like having two lines that start with "Use ..." ::: gedit/dialogs/gedit-preferences-dialog.c @@ +431,3 @@ + /* Bind widgets to settings keys */ + /* NOTE: This has to happen before setting the sensitivity of widgets, Is this true or for the current code? If yes, - can you describe how to observe the bug? - can you provide a separate patch with just this change? @@ +458,3 @@ + /* Read current config and setup initial state */ + if (use_default_font) { gedit coding style uses braces on separate lines, see the surrounding code for examples
> I see the argument about "mutual exclusion", on the other hand I always find > radio buttons with just two items a bit odd and I think the current ui makes a > better use of space since it does not have two "white circles". Regarding the two radiobuttons, in the GNOME HIG (http://developer.gnome.org/hig-book/2.91/controls-radio-buttons.html.en) it says at least two radio buttons is fine, and in fact radio buttons shouldn't be used when there are lots of items. While I see your point about make better use of the space, I don't think space is a major issue, as it doesn't look cramped or anything and I guess losing some space is a good sacrifice if it is clearer and easier to use. > That said I am open to this change if the UI people think it is a good idea. If > we merge it however we probably want to use different strings for the labels: I > do not like having two lines that start with "Use ..." Sure, but IIRC I think radio buttons should start with an imperative verb, however it doesn't have to be Use :) > Is this true or for the current code? If yes, > - can you describe how to observe the bug? > - can you provide a separate patch with just this change? No not for the current code, but when I start using the radiobuttons stuff then it does seem to be true. I will provide a patch that can show this behaviour later. > gedit coding style uses braces on separate lines, see the surrounding code for > examples Sorry about this I will sort it out :)
Created attachment 187288 [details] [review] Patch that fixes the issue #2 Here is an amended patch that formats the brackets in accordance with gedit's coding style
Created attachment 187290 [details] [review] Patch that doesn't work, to show why the GSettings binding code must come first And here is a patch to show why the GSettings binding code must come first
Mass-closing of all gedit bugzilla tickets. Special "code" to find again all those gedit bugzilla tickets that were open before the mass-closing: 2bfe1b0590a78457e1f1a6a90fb975f5878cb60064ccfe1d7db76ca0da52f0f3 By searching the above sha256sum in bugzilla, the gedit contributors can find again the tickets. We may be interested to do so when we work on a specific area of the code, to at least know the known problems and possible enhancements. We do this mass-closing because bugzilla.gnome.org is being replaced by gitlab.gnome.org.