Closed Bug 425916 Opened 12 years ago Closed 8 years ago

Improve look of mac treerows when in editing mode

Categories

(Toolkit :: Themes, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: stefanh, Assigned: stefanh)

Details

Attachments

(3 files, 2 obsolete files)

Spin-off from bug 420726, comment #12.

In summary (pinstripe/global/tree.css):

391 /* ::::: editable tree ::::: */
 392 
 393 treechildren::-moz-tree-row(selected, editing) {
 394   background-color: transparent;
 395 }
 396 
 397 treechildren::-moz-tree-cell-text(selected, editing) {
 398   color: inherit;
 399 }

Those rules doesn't work on odd rows, because of tree:not([hidecolumnpicker="true"]) treechildren::-moz-tree-row(odd) and treechildren[alternatingbackground="true"]::-moz-tree-row(odd) background color rules.

However, we might want to consider styling this so it'll match what Safari does in the bookmarks view (editable list rows, just click-hold on a bookmark). enn, does this sounds reasonable to you? I don't know a lot of editable trees, so I might have missed something here :-)
Component: XUL Widgets → Themes
QA Contact: xul.widgets → themes
Attached patch v1.0 (obsolete) — Splinter Review
So, I think what's left here is to fix the actual editing mode. Right now, the highlight color makes it look quite funny in the editbookmark panel when you have changed the selection color in system prefs.

Apple's rows are not as hight as ours, so compared to Safari (edit a bookmark title) we have more space above the text. The space is needed to extend the editable field to the top of the row. When playing with this, I used the editable tree example at https://wiki.mozilla.org/XUL:Tree with dom.allow_XUL_XBL_for_file set to true.

Markus, I'm not sure Dao has a mac. Would you mind giving some feedback?
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
Attachment #584286 - Flags: feedback?(mstange)
Attached image before-after (obsolete) —
Hmm, did it became better? OK, we're using the correct color. But I dunno about the positioning (margin/padding).
Comment on attachment 584286 [details] [diff] [review]
v1.0

Hm, I think I'll try to see if I can make it better...
Attachment #584286 - Flags: feedback?(mstange)
Attached file XUL test file
Here's a test file to be used locally (set 'dom.allow_XUL_XBL_for_file' to true).
Attached patch V1.1Splinter Review
Markus, maybe you could take a look at this? The treechildren rules should not be needed (and we don't want the text color to inherit since we only want the black text color in the input box - see the testcase).
Attachment #584286 - Attachment is obsolete: true
Attachment #584287 - Attachment is obsolete: true
Attachment #588686 - Flags: review?(mstange)
Comment on attachment 588686 [details] [diff] [review]
V1.1

Looks good to me. But Dão should have a look, too.
Attachment #588686 - Attachment is patch: true
Attachment #588686 - Flags: review?(mstange)
Attachment #588686 - Flags: review?(dao)
Attachment #588686 - Flags: review+
Comment on attachment 588686 [details] [diff] [review]
V1.1

screenshot?
Attached image Screenshots
Attachment #588686 - Flags: review?(dao) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/e57b01411394
Whiteboard: [inbound]
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/e57b01411394
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
You need to log in before you can comment on or make changes to this bug.