unofficial mirror of bug-gnu-emacs@gnu.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; 10+ 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] 10+ messages in thread

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-03-22 15:33 UTC (permalink / raw)
  To: Stephen Berman, Mauro Aranda; +Cc: 69942

> Date: Fri, 22 Mar 2024 16:04:15 +0100
> From:  Stephen Berman via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 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?

Adding Mauro to the discussion.





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

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Mauro Aranda @ 2024-03-23 21:05 UTC (permalink / raw)
  To: Eli Zaretskii, Stephen Berman; +Cc: 69942

Stephen Berman <stephen.berman@gmx.net> writes:

 > 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?

Thanks for working on this.  What about adding a widget-unselected face?
I think that might be the intention with using the widget-inactive face
for unselected radio items.






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

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-03-24 18:47 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 69942, Eli Zaretskii

On Sat, 23 Mar 2024 18:05:30 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> 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?
>
> Thanks for working on this.  What about adding a widget-unselected face?
> I think that might be the intention with using the widget-inactive face
> for unselected radio items.

Yes, I agree that was likely the intention.  But I think it's
superfluous: after all, the distinction between selected (or chosen) and
unselected items is already clear from the appearance of the radio
buttons or, with checklist widgets, the check boxes (my patch neglected
checklists, but it's straightforward to account for them: in
widget-checklist-add-item the (widget-apply child :deactivate) sexp
should be wrapped in an (unless widget-radio-face-from-state ...)).

On the other hand, with an unselected face for the labels of the radio
button or check boxes, if it defaults to inheriting the shadow face for
unselected items, that corresponds to the current appearance with the
widget-inactive face, and by setting the widget-unselected face to the
default face, all labels would appear the same, which is what I want.
So for me that's an acceptable alternative to my proposed defcustom.  I
tried to implement it, but I'm not very conversant with the workings of
widget properties and how to apply faces depending on the widget's
state, and I haven't managed to come up with a working implementation
yet.  I'll keep trying, but you or someone else might be able to do it
sooner.

(There is another argument, besides superfluousness, against using a
separate face for unselected items: using multiple check boxes instead
of a checklist, as e.g. recentf-edit-list does.  With these the label of
each check box is supplied by the :tag property, so it is not touched by
the current handling in terms of the child widget's activation state.
I'm not sure if using an unselected face here would be unproblematic or
not.)

Steve Berman





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

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-03-25  0:40 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 69942, Eli Zaretskii

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

On Sun, 24 Mar 2024 19:47:16 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Sat, 23 Mar 2024 18:05:30 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:
>
>> Stephen Berman <stephen.berman@gmx.net> writes:
>>
>>> 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?
>>
>> Thanks for working on this.  What about adding a widget-unselected face?
>> I think that might be the intention with using the widget-inactive face
>> for unselected radio items.
>
> Yes, I agree that was likely the intention.  But I think it's
> superfluous: after all, the distinction between selected (or chosen) and
> unselected items is already clear from the appearance of the radio
> buttons or, with checklist widgets, the check boxes (my patch neglected
> checklists, but it's straightforward to account for them: in
> widget-checklist-add-item the (widget-apply child :deactivate) sexp
> should be wrapped in an (unless widget-radio-face-from-state ...)).
>
> On the other hand, with an unselected face for the labels of the radio
> button or check boxes, if it defaults to inheriting the shadow face for
> unselected items, that corresponds to the current appearance with the
> widget-inactive face, and by setting the widget-unselected face to the
> default face, all labels would appear the same, which is what I want.
> So for me that's an acceptable alternative to my proposed defcustom.  I
> tried to implement it, but I'm not very conversant with the workings of
> widget properties and how to apply faces depending on the widget's
> state, and I haven't managed to come up with a working implementation
> yet.  I'll keep trying, but you or someone else might be able to do it
> sooner.
>
> (There is another argument, besides superfluousness, against using a
> separate face for unselected items: using multiple check boxes instead
> of a checklist, as e.g. recentf-edit-list does.  With these the label of
> each check box is supplied by the :tag property, so it is not touched by
> the current handling in terms of the child widget's activation state.
> I'm not sure if using an unselected face here would be unproblematic or
> not.)

Ok, I've gotten further with implementing disinguishing by faces
selected (chosen) and unselected radio buttons in radio-button-choice
widgets and check boxes in checklist widgets, see the attached patch.
Initial tests seem ok, but it definitely needs more testing.

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: selected and unselected widgets --]
[-- Type: text/x-patch, Size: 5205 bytes --]

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 172da3db1e0..005aa918087 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -555,6 +555,27 @@ widget-specify-active
       (delete-overlay inactive)
       (widget-put widget :inactive nil))))

+(defface widget-unselected
+  '((t :inherit shadow))
+  "Face used for unselected widgets."
+  :group 'widget-faces
+  :version "30.1")
+
+(defun widget-specify-unselected (widget from to)
+  "Fontify WIDGET as unselected (not chosen)."
+  (let ((overlay (make-overlay from to nil t nil)))
+    (overlay-put overlay 'face 'widget-unselected)
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'priority 100)
+    (widget-put widget :unselected overlay)))
+
+(defun widget-specify-selected (widget)
+  "Remove fontification of WIDGET as unselected (not chosen)."
+  (let ((unselected (widget-get widget :unselected)))
+    (when unselected
+      (delete-overlay unselected)
+      (widget-put widget :unselected nil))))
+
 ;;; Widget Properties.

 (defsubst widget-type (widget)
@@ -2415,10 +2436,16 @@ 'checkbox
 (defun widget-checkbox-action (widget &optional event)
   "Toggle checkbox, notify parent, and set active state of sibling."
   (widget-toggle-action widget event)
-  (let ((sibling (widget-get-sibling widget)))
+  (let* ((sibling (widget-get-sibling widget))
+         (from (widget-get sibling :from))
+	 (to (widget-get sibling :to)))
     (when sibling
-      (widget-apply sibling
-	            (if (widget-value widget) :activate :deactivate))
+      (if (widget-value widget)
+          (progn
+            (widget-apply sibling :activate)
+            (widget-specify-selected sibling))
+        :deactivate
+        (widget-specify-unselected sibling from to))
       (widget-clear-undo))))

 ;;; The `checklist' Widget.
@@ -2474,15 +2501,19 @@ widget-checklist-add-item
 	       ((eq escape ?v)
 		(setq child
 		      (cond ((not chosen)
-			     (let ((child (widget-create-child widget type)))
-			       (widget-apply child :deactivate)
+			     (let* ((child (widget-create-child widget type))
+                                    (from (widget-get child :from))
+			            (to (widget-get child :to)))
+                               (widget-specify-unselected child from to)
 			       child))
                             ((widget-inline-p type t)
 			     (widget-create-child-value
-			      widget type (cdr chosen)))
+			      widget type (cdr chosen))
+                             (widget-specify-selected child))
 			    (t
 			     (widget-create-child-value
-			      widget type (car (cdr chosen)))))))
+			      widget type (car (cdr chosen)))
+                             (widget-specify-selected child)))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2653,8 +2684,11 @@ widget-radio-add-item
 				(widget-create-child-value
 				 widget type value)
 			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+                (if chosen
+                    (widget-specify-selected child)
+                  (let ((from (widget-get child :from))
+			(to (widget-get child :to)))
+                    (widget-specify-unselected child from to))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2704,14 +2738,17 @@ widget-radio-value-set
     (dolist (current (widget-get widget :children))
       (let* ((button (widget-get current :button))
 	     (match (and (not found)
-			 (widget-apply current :match value))))
+			 (widget-apply current :match value)))
+             (from (widget-get current :from))
+	     (to (widget-get current :to)))
 	(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))))))
+            (progn
+              (widget-value-set current value)
+              (widget-apply current :activate)
+              (widget-specify-selected current))
+          (widget-specify-unselected current from to))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2731,13 +2768,16 @@ widget-radio-action
   (let ((buttons (widget-get widget :buttons)))
     (when (memq child buttons)
       (dolist (current (widget-get widget :children))
-	(let* ((button (widget-get current :button)))
+	(let* ((button (widget-get current :button))
+               (from (widget-get current :from))
+	       (to (widget-get current :to)))
 	  (cond ((eq child button)
 		 (widget-value-set button t)
-		 (widget-apply current :activate))
+		 (widget-apply current :activate)
+                 (widget-specify-selected current))
 		((widget-value button)
 		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+                 (widget-specify-unselected current from to)))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))


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

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-01 15:21 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 69942, Eli Zaretskii

On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Sun, 24 Mar 2024 19:47:16 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:
>
>> On Sat, 23 Mar 2024 18:05:30 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:
>>
>>> Stephen Berman <stephen.berman@gmx.net> writes:
>>>
>>>> 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?
>>>
>>> Thanks for working on this.  What about adding a widget-unselected face?
>>> I think that might be the intention with using the widget-inactive face
>>> for unselected radio items.
>>
>> Yes, I agree that was likely the intention.  But I think it's
>> superfluous: after all, the distinction between selected (or chosen) and
>> unselected items is already clear from the appearance of the radio
>> buttons or, with checklist widgets, the check boxes (my patch neglected
>> checklists, but it's straightforward to account for them: in
>> widget-checklist-add-item the (widget-apply child :deactivate) sexp
>> should be wrapped in an (unless widget-radio-face-from-state ...)).
>>
>> On the other hand, with an unselected face for the labels of the radio
>> button or check boxes, if it defaults to inheriting the shadow face for
>> unselected items, that corresponds to the current appearance with the
>> widget-inactive face, and by setting the widget-unselected face to the
>> default face, all labels would appear the same, which is what I want.
>> So for me that's an acceptable alternative to my proposed defcustom.  I
>> tried to implement it, but I'm not very conversant with the workings of
>> widget properties and how to apply faces depending on the widget's
>> state, and I haven't managed to come up with a working implementation
>> yet.  I'll keep trying, but you or someone else might be able to do it
>> sooner.
>>
>> (There is another argument, besides superfluousness, against using a
>> separate face for unselected items: using multiple check boxes instead
>> of a checklist, as e.g. recentf-edit-list does.  With these the label of
>> each check box is supplied by the :tag property, so it is not touched by
>> the current handling in terms of the child widget's activation state.
>> I'm not sure if using an unselected face here would be unproblematic or
>> not.)
>
> Ok, I've gotten further with implementing disinguishing by faces
> selected (chosen) and unselected radio buttons in radio-button-choice
> widgets and check boxes in checklist widgets, see the attached patch.
> Initial tests seem ok, but it definitely needs more testing.

Any comments on this patch for using a widget-unselected face?  I have
been detained from further testing this past week, but can now resume.

Steve Berman





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

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-04-06  9:02 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 69942, maurooaranda

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  69942@debbugs.gnu.org
> Date: Mon, 01 Apr 2024 17:21:27 +0200
> 
> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:
> 
> > Ok, I've gotten further with implementing disinguishing by faces
> > selected (chosen) and unselected radio buttons in radio-button-choice
> > widgets and check boxes in checklist widgets, see the attached patch.
> > Initial tests seem ok, but it definitely needs more testing.
> 
> Any comments on this patch for using a widget-unselected face?  I have
> been detained from further testing this past week, but can now resume.

Mauro, any further comments?





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

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  0 siblings, 2 replies; 10+ messages in thread
From: Mauro Aranda @ 2024-04-08 10:58 UTC (permalink / raw)
  To: Eli Zaretskii, Stephen Berman; +Cc: 69942

On 6/4/24 06:02, Eli Zaretskii wrote:
 >> From: Stephen Berman <stephen.berman@gmx.net>
 >> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
 >> Date: Mon, 01 Apr 2024 17:21:27 +0200
 >>
 >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman 
<stephen.berman@gmx.net> wrote:
 >>
 >>> Ok, I've gotten further with implementing disinguishing by faces
 >>> selected (chosen) and unselected radio buttons in radio-button-choice
 >>> widgets and check boxes in checklist widgets, see the attached patch.
 >>> Initial tests seem ok, but it definitely needs more testing.
 >>
 >> Any comments on this patch for using a widget-unselected face?  I have
 >> been detained from further testing this past week, but can now resume.
 >
 > Mauro, any further comments?

Hi Eli and Stephen,

Please forgive me, for the past 2 weeks I haven't been able to do any
computer stuff.  If it's OK, please give me until the weekend so I
can catch up with this and the other 2 bug reports by Stephen.





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

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  1 sibling, 0 replies; 10+ messages in thread
From: Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-08 11:15 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 69942, Eli Zaretskii

On Mon, 8 Apr 2024 07:58:44 -0300 Mauro Aranda <maurooaranda@gmail.com> wrote:

> On 6/4/24 06:02, Eli Zaretskii wrote:
>>> From: Stephen Berman <stephen.berman@gmx.net>
>>> Cc: Eli Zaretskii <eliz@gnu.org>, 69942@debbugs.gnu.org
>>> Date: Mon, 01 Apr 2024 17:21:27 +0200
>>>
>>> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman <stephen.berman@gmx.net>
>   wrote:
>>>
>>>> Ok, I've gotten further with implementing disinguishing by faces
>>>> selected (chosen) and unselected radio buttons in radio-button-choice
>>>> widgets and check boxes in checklist widgets, see the attached patch.
>>>> Initial tests seem ok, but it definitely needs more testing.
>>>
>>> Any comments on this patch for using a widget-unselected face?  I have
>>> been detained from further testing this past week, but can now resume.
>>
>> Mauro, any further comments?
>
> Hi Eli and Stephen,
>
> Please forgive me, for the past 2 weeks I haven't been able to do any
> computer stuff.  If it's OK, please give me until the weekend so I
> can catch up with this and the other 2 bug reports by Stephen.

Please take your time; I'm in no rush.

Steve Berman





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

* bug#69942: 30.0.50; Fontification of radio-button widget labels
  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
  1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2024-04-08 12:03 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 69942, stephen.berman

> Date: Mon, 8 Apr 2024 07:58:44 -0300
> Cc: 69942@debbugs.gnu.org
> From: Mauro Aranda <maurooaranda@gmail.com>
> 
>  > Mauro, any further comments?
> 
> Hi Eli and Stephen,
> 
> Please forgive me, for the past 2 weeks I haven't been able to do any
> computer stuff.  If it's OK, please give me until the weekend so I
> can catch up with this and the other 2 bug reports by Stephen.

No need to apologize, we all have our Real Lives.

Take your time, there's no urgency.  Thanks in advance.





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

end of thread, other threads:[~2024-04-08 12:03 UTC | newest]

Thread overview: 10+ 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

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).