Closed
Bug 372014
Opened 17 years ago
Closed 17 years ago
Unify Sunbird/Lightning preferences code
Categories
(Calendar :: Preferences, defect)
Calendar
Preferences
Tracking
(Not tracked)
VERIFIED
FIXED
Lightning 0.5
People
(Reporter: ssitter, Assigned: ssitter)
References
Details
Attachments
(2 files, 1 obsolete file)
63.54 KB,
image/png
|
mvl
:
ui-review+
|
Details |
39.18 KB,
patch
|
mattwillis
:
first-review+
|
Details | Diff | Splinter Review |
Instead of adding new preferences UI tin Lightning we should unify Sunbird/Lightning preferences code. Goal of this bug is to reuse the existing preference UI from mozilla/calendar/base/content/preferences in Lightning.
Assignee | ||
Comment 1•17 years ago
|
||
Changes contained in patch: - Remove the existing Lightning preference UI - Add General/Alarms/Categories/Views/Timezone tab to Lightning pref. pane - Add empty box object to Sunbirds pref. panes / Lightnings pref. tabs - Overlay the box object with the existing preference overlays from Sunbird - Add some missing default prefs to Lightning and do some code adaption to make the preferences work in Lightning Patch could only been tested with Sunbird/0.5pre and Thunderbird/2.0pre + Lightning/0.5pre builds on Windows 2000.
Attachment #256696 -
Flags: first-review?(lilmatt)
Assignee | ||
Comment 2•17 years ago
|
||
(Don't blame me for Windows 2000 being so ugly. Beautification can be done later)
Attachment #256697 -
Flags: ui-review?(mvl)
Comment 3•17 years ago
|
||
This patch means that we can not use tabs in sunbird preference pages. I'm not sure if that's a bad thing, but we must be aware if it. (there is no technical reason for this, but tabs in tabs are just plain ugly and very confusing)
Comment 4•17 years ago
|
||
True. We currently only use tabs on the "Advanced" pane, which I'm assuming we don't need to port to Ln.
Assignee | ||
Comment 5•17 years ago
|
||
(In reply to comment #3) > This patch means that we can not use tabs in sunbird preference pages. Sure you can. If we want to use tabs in Sunbird in future: Do not overlay the preference pane anymore. Instead add tabs to the preference pane. Overlay the tabs similar to how it's done in Lightning. In Lightning just add new tabs and overlay it too. (In reply to comment #4) > True. We currently only use tabs on the "Advanced" pane, which I'm > assuming we don't need to port to Ln. This is right, Advanced pane is not ported to Lightning. But it is included in this patch to make the handling for all panes in Sunbird equal.
Comment 6•17 years ago
|
||
Comment on attachment 256696 [details] [diff] [review] rev0 - unify preferences >Index: mozilla/calendar/base/content/preferences/preferences.xul >=================================================================== > <prefpane id="paneGeneral" > label="&paneGeneral.title;" >- src="chrome://calendar/content/preferences/general.xul"/> >+ onpaneload="gGeneralPane.init();" >+ src="chrome://calendar/content/preferences/general.xul"> >+ <vbox id="calPreferencesBoxGeneral"/> >+ </prefpane> This vbox tag and all the others should be indented 2 more spaces. The file is uses 4 space indenting. >Index: mozilla/calendar/base/content/preferences/categories.js >=================================================================== > init: function () { > // On non-instant-apply platforms, once this pane has been loaded, > // attach our "revert all changes" function to the parent prefwindow's > // "ondialogcancel" event. > var parentPrefWindow = document.getElementById("CalendarPreferences"); >+ if (!parentPrefWindow) { >+ parentPrefWindow = document.getElementById("MailPreferences"); >+ } I'd prefer: var parentPrefWindow = document.getElementById("CalendarPreferences") || document.getElementById("MailPreferences"); >Index: mozilla/calendar/lightning/jar.mn >=================================================================== >- content/lightning/lightning-preferences.xul (content/lightning-preferences.xul) >- content/lightning/lightning-preferences.js (content/lightning-preferences.js) Note that these files should also be cvs rm'd when this patch lands. >Index: mozilla/calendar/lightning/content/messenger-overlay-preferences.xul >=================================================================== >-<!DOCTYPE overlay SYSTEM "chrome://lightning/locale/lightning.dtd" > >+<!DOCTYPE overlay [ >+ <!ENTITY % lightningDTD SYSTEM "chrome://lightning/locale/lightning.dtd"> >+ <!ENTITY % preferencesDTD SYSTEM "chrome://calendar/locale/preferences/preferences.dtd"> >+ %lightningDTD; >+ %preferencesDTD; >+]> Don't line up the SYSTEMs. If we have a reallyLongThingDTD in the future it will encourage editing all three lines simply for alignment and unnecessarily pollute cvs blame. > <prefwindow id="MailPreferences"> >- <prefpane id="paneLightning" label="&lightning.taskLabel;" >- src="chrome://lightning/content/lightning-preferences.xul"/> >+ <prefpane id="paneLightning" label="&lightning.taskLabel;" >+ onpaneload="gGeneralPane.init(); gAlarmsPane.init(); gCategoriesPane.init(); gTimezonesPane.init();"> Please move label= to its own line >+ <tabbox id="calPreferencesTabbox" flex="1" onselect=""> >+ <tabs> >+ <tab id="calPreferencesTabGeneral" >+ label="&paneGeneral.title;"/> >+ <tab id="calPreferencesTabAlarms" >+ label="&paneAlarms.title;"/> >+ <tab id="calPreferencesTabCategories" >+ label="&paneCategories.title;"/> >+ <tab id="calPreferencesTabViews" >+ label="&paneViews.title;"/> >+ <tab id="calPreferencesTabTimezones" >+ label="&paneTimezones.title;"/> >+ </tabs> >+ <tabpanels flex="1"> >+ <tabpanel orient="vertical"> >+ <vbox id="calPreferencesBoxGeneral"/> >+ </tabpanel> >+ <tabpanel orient="vertical"> >+ <vbox id="calPreferencesBoxAlarms"/> >+ </tabpanel> >+ <tabpanel orient="vertical"> >+ <vbox id="calPreferencesBoxCategories"/> >+ </tabpanel> >+ <tabpanel orient="vertical"> >+ <vbox id="calPreferencesBoxViews"/> >+ </tabpanel> >+ <tabpanel orient="vertical"> >+ <vbox id="calPreferencesBoxTimezones"/> >+ </tabpanel> >+ </tabpanels> >+ </tabbox> Should we be persisting which tab was selected so we show it again next time the user opens the prefs? >+ </prefpane> > </prefwindow> Since you're touching everything here, fix the indenting of the <prefwindow> tag as well. > </overlay> >\ No newline at end of file Fix that. >Index: mozilla/calendar/lightning/content/lightning.js >=================================================================== >+// categories settings >+// XXX one day this should contain a list of locale specific default categories >+pref("calendar.categories.names", ""); One day? Why are we not using the ones from sunbird.js? I'd like to see this one more time before checking it in.
Attachment #256696 -
Flags: first-review?(lilmatt) → first-review-
Comment 7•17 years ago
|
||
Comment on attachment 256697 [details]
rev0 - screenshot of Lightning preferences
ui-review=mvl
Attachment #256697 -
Flags: ui-review?(mvl) → ui-review+
Assignee | ||
Comment 8•17 years ago
|
||
Same as rev0 but with the issues from Comment #6 addressed except: >>Index: mozilla/calendar/lightning/content/messenger-overlay-preferences.xul >Should we be persisting which tab was selected so >we show it again next time the user opens the prefs? Thunderbird/Sunbird use javascript foo to save/restore the tab order from/to a preference. I don't want to enlarge the patch unnecessary therefore this will be handled in a followup bug as discussed on IRC. >>Index: mozilla/calendar/lightning/content/lightning.js >>+pref("calendar.categories.names", ""); >One day? Why are we not using the ones from sunbird.js? Currently the Lightning default preferences are not locale specific. Therefore I thought it would be better to ship no default categories and let the user create them as needed instead of shipping English categories for all locales.
Attachment #256696 -
Attachment is obsolete: true
Attachment #257759 -
Flags: first-review?(lilmatt)
Comment 9•17 years ago
|
||
Comment on attachment 257759 [details] [diff] [review] rev1 - unify preferences r=lilmatt
Attachment #257759 -
Flags: first-review?(lilmatt) → first-review+
Comment 10•17 years ago
|
||
Patch checked in on MOZILLA_1_8_BRANCH and trunk. -> FIXED Nice work!
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
OS: Windows 2000 → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → Lightning 0.5
Updated•17 years ago
|
Status: RESOLVED → VERIFIED
Comment 12•17 years ago
|
||
(In reply to comment #8) > >Should we be persisting which tab was selected so > >we show it again next time the user opens the prefs? > > Thunderbird/Sunbird use javascript foo to save/restore the tab order from/to a > preference. I don't want to enlarge the patch unnecessary therefore this will > be handled in a followup bug as discussed on IRC. Stefan, I can't find the follow-up bug you spoke of here. Any hints?
Updated•17 years ago
|
Component: General → Preferences
QA Contact: general → preferences
Assignee | ||
Comment 13•17 years ago
|
||
(In reply to comment #12) Bug 373209
You need to log in
before you can comment on or make changes to this bug.
Description
•