Closed
Bug 79258
Opened 23 years ago
Closed 23 years ago
Message Filters: UI Cleanup
Categories
(MailNews Core :: Filters, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.2
People
(Reporter: jglick, Assigned: hwaara)
References
Details
(Keywords: polish, Whiteboard: r=bhuvan, sr=hewitt, waiting for trunk unfreeze)
Attachments
(4 files)
Some minor cleanup items. 1. The "Filters for:" menu should appear to the right of its label, not below it. 2. "Description" should be replaced with "Filter Name" (since that is what is being displayed). 3. The second column should be titled "Enabled". Currently it is blank. 4. A checkbox (see Subscription dialog) should be used under the "Enabled" column, not the green star. The green star means read/unread. 5. The checkbox (#4) should be centered under the "Enabled" column. 6. The descriptive text should be "Enabled filters are run automatically in the order shown below." 7. Are the arrows on the Move Up/Move Down buttons necessary? They make all the other buttons wider and aren't very attractive visually.
Just for reference, the UI cleanup bug for Filter Rules is bug 45038
QA Contact: esther → laurel
Assignee | ||
Comment 2•23 years ago
|
||
Taking, I'm working on a fix.
Assignee: naving → hwaara
Keywords: ui
Comment 3•23 years ago
|
||
hwaara, to speed up the review process, please break up your fix in pieces. as you know, smaller fixes can get reviewed / tested faster. take a single task on this list, attach a patch, get it reviewed and landed (and out of your tree), and repeat. we can keep this bug open as long as we need to, and continue to check in your improvements in increments. divide and conquer!
http://www.mozilla.org/mailnews/specs/filters/images/FilterA1.gif Note: "Run Filter(s) Now" (hence text: "Filters can also be run manually by highlighting the filter(s) and selecting "Run Filter(s) Now") and "Filter Log" features are not currently implemented.
Assignee | ||
Comment 5•23 years ago
|
||
Hey, I have all the nits plus some more (except for the checkbox vs. green thing image) fixed. I don't know whether that is unfortunate or not. Seth, is it OK?
Assignee | ||
Comment 7•23 years ago
|
||
Assignee | ||
Comment 8•23 years ago
|
||
Jennifer, Scott, Seth, Anyone: The latest screenshot includes all fixes originally pointed out except for the green image. What do you all think? Any other things you want fixed, UI-wise? My only current nit is that the dialog is too high. I will fix that before I attach a patch here.
Nice job! Thanks. :-) File a separate bug on the read/unread green stars being replaced with checkboxes issue if nec, thanks.
Comment 10•23 years ago
|
||
That looks really nice. There's another bug out there (doesn't need to go with this bug, but I thought you might be interested in investigating since you are already in this dialog) that would also make this window nicer. See http://bugzilla.mozilla.org/show_bug.cgi?id=69860
Comment 11•23 years ago
|
||
if you've already got a patch, great! attach it and we'll work on getting it in for 0.9.2 (too late for 0.9.1) next time you've got a bug like this, feel free to divide and conquer to speed up the process.
Assignee | ||
Comment 12•23 years ago
|
||
Assignee | ||
Comment 13•23 years ago
|
||
None of you people acually noticed that I missed a few wording changes in the last screenshot. :) Anyway, those are fixed now and I am satisfied with the height. So here's what my coming patch fixes: * The columns are now resizable * All jglick's nits * Renamed "Delete" to "Delete..." because it brings up a dialog * Removed lots of old code cruft Patch coming up.
Assignee | ||
Comment 14•23 years ago
|
||
Assignee | ||
Comment 15•23 years ago
|
||
bug 82754 filed about the checkbox icon.
Comment 16•23 years ago
|
||
sr=hewitt
Reporter | ||
Comment 17•23 years ago
|
||
"Delete" shouldn't be "Delete...". Elipsis are only used when additional input from the user is needed. Confirmation dialogs are not included in that category. Not necessary here. None of our other Delete buttons have elipsis.
Assignee | ||
Comment 18•23 years ago
|
||
Bhuvan can you test and review this patch? Thanks! Jglick: I will fix that before I land my patch. The new bug about checkbox-image is 82758. Scratch that earlier comment.
OS: Windows 98 → All
Assignee | ||
Comment 19•23 years ago
|
||
*** Bug 43697 has been marked as a duplicate of this bug. ***
Comment 20•23 years ago
|
||
reviewed and tested the patch. Looks good. nice work. Only thing I want to suggest is that it will be nice if we can persist on the window sizes (height, width) incase user resizes the window. It's not a major issue..but it is a pretty simple fix. Two additional items this patch to enable the persistence are id="filterListDialog" and persist="width height". +<window class="dialog" id="filterListDialog" + xmlns:NC="http://home.netscape.com/NC-rdf#" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" orient="vertical" onload="onLoad();" - style="width: 34em;" + width="440" + height="320" + persist="width height" Please get some feedback on this. Then, if needed, alter the patch to incorporate the above. r=bhuvan ps : Don't forget to fix 'Delete..' issue before you checkin.
Assignee | ||
Comment 21•23 years ago
|
||
Thanks Bhuvan! I will incorporate your suggestions in my tree now, and then checkin with that. Now all I am waiting for is 0.9.2 tree opening...
Status: NEW → ASSIGNED
Whiteboard: r=bhuvan, sr=hewitt, waiting for trunk unfreeze
Target Milestone: --- → mozilla0.9.2
a=dbaron for trunk checkin assuming comments above are dealt with
Assignee | ||
Comment 23•23 years ago
|
||
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 24•23 years ago
|
||
eye balling what actually got checked in (which is not the same as the fix above), it looks like this line got removed from FilterListDialog.xul: <stringbundle id="bundle_filter" src="chrome://messenger/locale/filter.properties"/> that would break the confirmation code that gets run when you confirm the delete of a filter. I'll check my build, and if it's busted I'll fix it.
Assignee | ||
Comment 25•23 years ago
|
||
Oops (how'd that happen?). Sorry about that! r=hwaara on reverting that line back :(
Comment 26•23 years ago
|
||
no worries. I've fixed it by adding that line back to the .xul file. fix checked in.
Assignee | ||
Comment 27•23 years ago
|
||
Thank you.
Comment 28•23 years ago
|
||
OK using june14 commercial trunk builds: win98, mac OS 9.0 and linux rh6.2. Checked in both classic, new modern themes. Generally checked for changes mentioned. Any other issues will be logged separately. Marking verified.
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•