all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#69942: 30.0.50; Fontification of radio-button widget labels
@ 2024-03-22 15:04 Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-03-22 15:33 ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-03-22 15:04 UTC (permalink / raw)
  To: 69942

[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]

In bug#69941 I reported a faulty fontification of radio-button widgets
and noted in passing that the labels associated with the radio buttons
also have unexpected faces, namely, the widget-inactive face regardless
of whether the associated radio buttons are inactive or active (except
for the label of a radio button that has been pressed, which has the
default face).  While the faulty fontification discussed in bug#69941
appears to be a real bug, the widget-inactive face assigned to
radio-button labels is apparently by design -- it was present in the
initial commit of the widget library.  But this seems to me to have been
a UX mistake, since it effectively ignores the semantics implied by the
name widget-inactive.  I think a less surprising UI would be for the
labels to be fontified according to the widget's activation state:
default face when the widget is active and widget-inactive face when
it's inactive.  The attached patches provide two possible
implementations of this UI.

The first patch makes the change unconditionally, treating the current
fontification as a UI/UX bug.  But it may be argued that this aspect of
the widget UI should not be unconditionally changed, since it was
apparently a deliberate design choice and there have been (AFAIK) no
complaints about the semantic discrepancy till now.  The lack of
complaint could be because the widget-inactive face inherits the shadow
face, so it is not sharply different from the default face.  But if one
uses a very different face (as I did for illustrative purposes in
bug#69941), the inconsistency is very obvious and (IMO) jarring.
Nevertheless, to allow keeping the current fontification, the second
patch conditionalizes the change from the current fontification by means
of a user option (with the default being the current fontification).

Is either of these changes acceptable?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: widget-radio-nocust.diff --]
[-- Type: text/x-patch, Size: 1558 bytes --]

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 172da3db1e0..21848849ba5 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -2652,9 +2652,7 @@ widget-radio-add-item
 		(setq child (if chosen
 				(widget-create-child-value
 				 widget type value)
-			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+			      (widget-create-child widget type))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2706,12 +2704,8 @@ widget-radio-value-set
 	     (match (and (not found)
 			 (widget-apply current :match value))))
 	(widget-value-set button match)
-	(if match
-	    (progn
-	      (widget-value-set current value)
-	      (widget-apply current :activate))
-	  (widget-apply current :deactivate))
-	(setq found (or found match))))))
+	(when match (widget-value-set current value))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2733,11 +2727,9 @@ widget-radio-action
       (dolist (current (widget-get widget :children))
 	(let* ((button (widget-get current :button)))
 	  (cond ((eq child button)
-		 (widget-value-set button t)
-		 (widget-apply current :activate))
+		 (widget-value-set button t))
 		((widget-value button)
-		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+		 (widget-value-set button nil)))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: widget-radio-cust.diff --]
[-- Type: text/x-patch, Size: 2585 bytes --]

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 172da3db1e0..616286a817d 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -2591,6 +2591,16 @@ 'radio-button
   :off "( )"
   :off-glyph "radio")

+(defcustom widget-radio-face-from-state nil
+  "How to fontify the label of a radio button.
+If non-nil, use `widget-inactive' face for the label if the
+radio-button-choice widget is inactive and default face if it is active.
+If nil, use the default face for the label only if the associated radio
+button is pressed, otherwise use `widget-inactive' face."
+  :type 'boolean
+  :group 'widget-faces
+  :version "30.1")
+
 (defun widget-radio-button-notify (widget _child &optional event)
   ;; Tell daddy.
   (widget-apply (widget-get widget :parent) :action widget event))
@@ -2653,8 +2663,9 @@ widget-radio-add-item
 				(widget-create-child-value
 				 widget type value)
 			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+		(unless widget-radio-face-from-state
+                  (unless chosen
+		    (widget-apply child :deactivate))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2706,12 +2717,14 @@ widget-radio-value-set
 	     (match (and (not found)
 			 (widget-apply current :match value))))
 	(widget-value-set button match)
-	(if match
-	    (progn
-	      (widget-value-set current value)
-	      (widget-apply current :activate))
-	  (widget-apply current :deactivate))
-	(setq found (or found match))))))
+	(if widget-radio-face-from-state
+            (when match (widget-value-set current value))
+	  (if match
+              (progn
+                (widget-value-set current value)
+                (widget-apply current :activate))
+            (widget-apply current :deactivate)))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2734,10 +2747,12 @@ widget-radio-action
 	(let* ((button (widget-get current :button)))
 	  (cond ((eq child button)
 		 (widget-value-set button t)
-		 (widget-apply current :activate))
+                 (unless widget-radio-face-from-state
+		   (widget-apply current :activate)))
 		((widget-value button)
 		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+		 (unless widget-radio-face-from-state
+                   (widget-apply current :deactivate))))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))


[-- Attachment #4: Type: text/plain, Size: 745 bytes --]



In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.18.0) of 2024-03-22 built on strobelfs2
Repository revision: c1530a2e4973005633ebe00d447f1f3aa1200301
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101009
System Description: Linux From Scratch r12.0-112

Configured using:
 'configure -C --with-xwidgets 'CFLAGS=-Og -g3'
 PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER
PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM XWIDGETS GTK3 ZLIB

^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2024-06-26  6:56 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-22 15:04 bug#69942: 30.0.50; Fontification of radio-button widget labels Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-22 15:33 ` Eli Zaretskii
2024-03-23 21:05   ` Mauro Aranda
2024-03-24 18:47     ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-25  0:40       ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-01 15:21         ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-06  9:02           ` Eli Zaretskii
2024-04-08 10:58             ` Mauro Aranda
2024-04-08 11:15               ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-08 12:03               ` Eli Zaretskii
2024-04-18  9:23               ` Eli Zaretskii
2024-04-18 10:07                 ` Mauro Aranda
2024-04-18 13:37                   ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-21 19:45                     ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-26 12:47                       ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-09  7:22                         ` Eli Zaretskii
2024-05-25  7:41                           ` Eli Zaretskii
2024-05-25  9:29                             ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-08 11:48                               ` Eli Zaretskii
2024-06-26  6:56                                 ` Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.