44032 - Remove `Apply' button from Themes pref panel (apply on `Ok')

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect, P3)

Description

[johng]

Using the themes pref panel, when you select a different theme, the user 
tendency is to press the OK button, but that doesn't do anything (must "Apply 
Theme"). Concerns:

- may not want to apply the theme when you say "OK"
- to make OK work, must save state of selection (OK is a prefs total thing, not 
specific to the panel)
- later, apply themes may be more complex (can use a combo of many skins, 
different skins for different components)

German, what would you recommend for users?

Comment 1

[johng]

nsbeta3

Comment 2

[johng]

nsbeta3

Comment 3

[Blake Ross]

putting myself back as qa contact...somehow john's last changes removed me. 
dawn, any idea about that?  I've seen it quite a few times.

The Bugzilla notification from john's last change reads:

[e-mail] changed:

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
          Component|Skinability                 |Preferences
          QAContact|[e-mail]          |0



------- Additional Comments From [e-mail]  2000-06-27 17:46 -------
nsbeta3


Is 0 really a better QA contact than me??? :`-(

Comment 4

[sairuh (rarely reading bugmail)]

h'm sounds like bug 40884... lemme how/why, if it isn't!

*** This bug has been marked as a duplicate of 40884 ***

Comment 5

[Blake Ross]

Actually, I don't think so.  I think john's concern here is that pressing the 
OK button after you've selected a new theme won't apply the theme...rather, you 
need to specifically press the apply button, something that many users might 
forget to do (and thus they'll be confused when their theme doesn't change upon 
pressing OK)

re-dup if I'm wrong (sorry, in that case)

Comment 6

[german]

I agree with John's concerns OK should simply apply the Theme like others prefs 
work in Mozilla. Forwarding to Ben.

Comment 7

[johng]

assigning to german - please integrate this detail with the other UI changes you
want to make for the Themes Switching pref panel, then mark this as a dupe.  We
will discuss all the changes with Ben and see what we can and can not do.

Comment 8

[hangas]

Marking nsbeta3- as johng is using this as a request for spec update

Comment 9

[Kathleen :Brade]

I have also run into this UE-bug.  German--can we get a new spec on this?

Also, from http://www.macintouch.com/netscape6.html, here are comments from John
Groseclose:

Clicking "OK" after choosing a new "theme" doesn't work - you need to "apply"
the theme and then click "cancel" to get out of the "Preferences" window.

Comment 10

[Matthew T (active 1999-2002)]

So fixing an obvious bug in Mozilla's UI is being held up until whenever German 
produces an updated spec on a more general topic (the UI of the Themes panel as a 
whole) ... Is that really a good idea?

Comment 11

[Matthew T (active 1999-2002)]

*** Bug 44388 has been marked as a duplicate of this bug. ***

Comment 12

[Blake Ross]

What are we all waiting for?  German said on 7/26:

"I agree with John's concerns OK should simply apply the Theme like others 
prefs 
work in Mozilla. Forwarding to Ben."

Taking this...

Comment 13

[Blake Ross]

Attachment: [patch] beginning patch

Comment 14

[Blake Ross]

Comments/concerns regarding the patch:

- The prefs window won't close, presumably because of the following error:

JavaScript error:
chrome://global/content/nsWidgetStateManager.js line 105: target of 'in' operato
r must be an object

The line in question is: if( 'GetFields' in this.contentArea)

I think skin switching is tearing something down and screwing us in the 
process, because a dump of |this.contentArea| at that point yielded the 
following results:

[object Window]
[object Window]
[object Window]
[object Window]
undefined

Themes is the fourth panel, so after that point, this.contentArea (which is 
ultimately the ID of the that contains each pref pane) is suddenly undefined.  
If I remove the refreshSkins(), all is fine.  I'll talk to ben/hyatt for 
suggestions.  Bill, any ideas about what to do here?  Am I just doing something 
wrong?

- We don't really need to re-store the chromeRegistry each time GetFields() is 
called.  When and where can I globally store this?  I tried to do it in Startup
() and it failed.

Ben, can we remove the debug installSkin() now?  I don't think we'll be needing 
it again.

Comment 15

[Matthew T (active 1999-2002)]

Is your problem that after the user has clicked `Ok', you are trying to change 
the theme before actually closing the prefs dialog? Settings should only 
visibly change after the dialog has closed. (And by closing the prefs dialog 
before updating the theme, you would save having to redraw the dialog itself in 
the new theme.)

Comment 16

[Blake Ross]

I'm open to ideas, but I don't see how it's possible to change the theme after 
the dialog is closed...all the pref saving, etc is done and then, at the very 
end, the dialog is closed.

Comment 17

[Todd Pringle]

I have heard a lot of user feedback about this behavior, we definitely need to
fix it.  One problem with making the OK button have the same behavior as the
Apply Theme button is that it will apply a theme that you already have running -
i.e. I look at the themes pref, I'm running modern, decide I don't want to
change it, and hit okay.  Modern is applied again.  That's not good either.  Can
we prevent that?

Comment 18

[Blake Ross]

We have a bug somewhere out there on not reapplying a theme if it's already 
being used.

Comment 19

[Matthew T (active 1999-2002)]

That's bug 53712.

[resummarizing]

Comment 20

[Brian Haskin (Janzert)]

*** Bug 69128 has been marked as a duplicate of this bug. ***

Comment 21

[Maarten Afman]

What about having a seperate popup when clicking on a theme:
"Do you want to use this theme? yes/no/cancel"
Then you can disable the apply button altogether and you can mark this bug fixed.

No flame intended but it would be real nice to try to solve this kind of
usability problems (of which resolution could be realized without too much
technical impact) within a year of the first bug report.. (now wouldn't it?)

Comment 22

[timeless]

<2001-03-27 12:29>
no. correct solution is to require people to click <ok> or <apply> or whatever 
the button at the edge of the prefs dialog is. the <apply theme> button is a 
hack/kludge.

Comment 23

[Gervase Markham [:gerv]]

If you remove "Apply Theme" and ask the user to click on "OK", how does the user
preview a number of different themes before settling on one? It would require
reopening the prefs dialog multiple times.

(This assumes dynamic skin switching works, of course.)

Gerv

Comment 24

[timeless]

each of the switching panels should have a picture preview inline.  If this 
isn't good enough then we need to file a huge rfe for WordPerfect's 
RealTimePreview feature. Mozilla support for that should be ready by mozilla2.0 
given the current 1.0 arguement I can assure you that would be a long way off 
:-) 

Comment 25

[Marko Macek]

People often forget  to click the Apply "theme" button because it is too far
from the theme selector.  If it stays it should be more visible.

Also, the currently applied theme should be marked  somehow (perhaps bold)

Comment 26

[Blake Ross]

Attachment: patch

Comment 27

[Blake Ross]

Attachment: patch: improved

fix a few display issues in this panel.
add a link to themepark.

Comment 28

[Blake Ross]

fixed.

Comment 29

[Blake Ross]

Cc'ing Jatin.  Help needs to be updated to reflect this.

Comment 30

[Patty Mac]

Blake, on Mac OS 9.2 if you click on "Classic" in Theme preferences dialog,
there should be a link to themepark like "Get New Theme".

Comment 31

[sairuh (rarely reading bugmail)]

patty, i'm thinking that issue might be due to the "themes panel doesn't fit"
bug 112314.

Comment 32

[Patty Mac]

Verified the patch.

Comment 33

[Blake Ross]

Jatin: don't forget that help needs to be changed...

Comment 34

[Jatin Billimoria]

Yes, there is a separate bug (Bug 121694) for the help, which I am taking.
Thanks for the heads-up.