Comment 1 Thomas Schindl 2010-06-17 02:06:23 EDT

Should we probably create a new bundle which holds 4.0 SDK enhancements to the standard 3.6 functionality?

Comment 2 Thomas Schindl 2010-06-17 10:47:40 EDT

Created attachment 172128 [details]
Fixed Minor Issues

Bogdan and Oleg, I think you should review (did I get that right?) so that we can ship with this with RC0

Comment 3 Thomas Schindl 2010-06-17 10:48:44 EDT

Bogdan, Oleg please review and provide me feedback what you think about it.

Comment 4 Bogdan Gheorghe 2010-06-17 13:27:30 EDT

Tom, I took a look at this and applied the patch and it seemed to be OK. (There are some bugs on my part during an engine reset, but that has nothing to do with this). I wonder, if we want to have anything to display any style sheets that have been registered? Maybe not for RC0 but for RC1? (BTW, I just noticed a typo in ThemeEngine.registerStylsheet => missing an 'e' in Stylesheet).

Comment 5 Oleg Besedin 2010-06-17 14:20:03 EDT

Looks quite good. Some CSS changes apply in a weird way, but that's not a problem with the dialog.

One thing I noticed is that the changed preferences are not persisted on shutdown?

Also, the label "ThemeId":

	new Label(comp, SWT.NONE).setText("ThemeId"); //$NON-NLS-1$


this is a user-oriented screen, how about just "Theme"? (And should it be translatable?)

Comment 6 John Arthorne 2010-06-18 09:58:36 EDT

I am wondering if this combo box should just be integrated into ViewsPreferencePage, which is already in org.eclipse.ui.workbench which is forked for 4.0. Having a new preference page for a single preference seems overkill. 

That page already has "Theme" and "Presentation" combo boxes that are likely made redundant by the CSS theme mechanism. Unless anyone thinks we will get the old  presentation preference working for 4.0 (org.eclipse.ui.presentations), I suggest removing those old combos and replacing with this new one. Otherwise it will be really confusing for the end user to understand which of the various theme/presentation preferences is the real one.

Comment 7 Paul Webster 2010-06-18 10:08:43 EDT

(In reply to comment #6)
> That page already has "Theme" and "Presentation" combo boxes that are likely
> made redundant by the CSS theme mechanism. Unless anyone thinks we will get the

No, they won't be working.  We should remove them and only offer them the new option.

PW

Comment 8 Thomas Schindl 2010-06-20 06:46:36 EDT

Created attachment 172287 [details]
Patch to replace old Appearance Page

I agree with John and Paul so this patch replaces the old page with the new one

Comment 9 Thomas Schindl 2010-06-20 06:59:53 EDT

Created attachment 172288 [details]
Patch to fix Typo

This patches fixes the typo mentionned by Bogdan. The problem of not persisting the preference on shutdown should have been fixed before.

Comment 10 Thomas Schindl 2010-06-21 08:22:03 EDT

Created attachment 172321 [details]
patch

I released the typo fix before readin the release plan which requires a +1 for such changes (sorry). 

So here's the proposed patch for the Preference Page replacement. Waiting for a +1

Comment 11 Thomas Schindl 2010-06-22 14:08:55 EDT

anyone a +1?

Comment 12 Boris Bokowski 2010-06-22 17:54:13 EDT

sure, +1

Comment 13 Thomas Schindl 2010-06-23 07:22:11 EDT

released new page to HEAD