Betterbird / thunderbird-patches

Betterbird is a fork of Mozilla Thunderbird. Here are the patches that provide all the goodness.
Other
468 stars 20 forks source link

Small issues with table view (115.2.0-bb10) #191

Closed VOOM108 closed 1 year ago

VOOM108 commented 1 year ago

There are 3 issues I have with the current implementation of the table view:

  1. When selecting a message in a conversation, it is highlighted. But if I click into the message content (or anywhere else) that highlight is gone. With long conversations, where I need to look through several mails, I am not sure where I am currently and tend to check the same messages over again. Expected behavior: Highlight remains when clicking into the highlighted email (e.g. for selecting and copying text) or some other way of visualizing where I am currently at.

  2. The open/closed thread state of conversations is always reset when changing to a different account or folder and going back. I believe to remember it used to remember the state of individual threads. Expected behavior: The open/closed state of individual conversations is saved until I explicitly act to change the individual state (or all states of all threads)

  3. The indentation of the messages in a conversation is so large, that the metadata of the messages is not visible anymore too early. E.g. depending on the width of the message list, the name of the author is hidden with the 4th message already. The room I can make for it with the width of the date column is limited. Expected behavior: Ideally, the indentation would change with the width of the message list. Or it could be set to different width by an option. In general, I think that the indentation should be as narrow as possible, because not much is needed to visualize what belongs to what. This may affect the connector line and also the "replied-icon", but maybe they could be placed differently. ATM there is an equally wide indentation before the line, for the line itself and for the "replied-icon". Maybe the first indentation could be removed, the line indentation become smaller and the replied-icon could be omitted in thread-view altogether, because the fact, that there is an indented message beneath already shows, that there is a reply. It is not really significant in this case.

Here is an extreme example, that shows how it can quickly become confusing esp. with the combination of 1 and 3:

image
Betterbird commented 1 year ago

I don't see 1. (using https://www.betterbird.eu/expert-tips/userChrome-115.css) 2. is a TB issue (we'll find the bug later). We'll comment on 3. later.

VOOM108 commented 1 year ago

I did not use that CSS; implemented it now to both userChrome.css in Local and Roaming and it is used, since I now see the label for the header info in red. It still does not show any highlight when the message list is out of focus, though.

Betterbird commented 1 year ago

Which platform and theme are you using?

There is provision to semi-highlight selected rows when the thread pane (message list) isn't focused. I can supply a screenshot. The question is why it doesn't work for you. Try a different theme.

BTW, our userChrome.css will work poorly for dark mode.

VOOM108 commented 1 year ago

(Windows 11) I was using a theme "Windows 11 Dark". I now switched to the "System Theme - auto" and there are some strange things happening: After switching the theme, the dimmed highlight is there as it should be. The buttons in the button bar on top are moved from left to center, not using the spacer between the left icons and the center icons. When I close Betterbird now and start it again, the buttons are correct, but the dimmed highlight is gone. I can't seem to get a consistent view...

Betterbird commented 1 year ago

How does Thunderbird 115 behave? We can file an upstream bug.

VOOM108 commented 1 year ago

I don't have TB115 installed.

I noticed something just now: when I go to manage themes, sometimes no theme shows as enabled. Right now, two themes show as enabled. The colors and highlights are correct when only one or no theme shows as enabled, but then the buttons are in the wrong place. The other way around, which is the state it always goes back to after the next restart of the app, the buttons are in the right place, but the dimmed highlight is gone. Some bug in the way the themes are handled? Is there a theme cache I could flush or something like that?

VOOM108 commented 1 year ago

I have deleted all themes now except the ones coming with TB. The buttons appear randomly in the center or the left. Also, except right after switching the theme, the dimmed highlight remains gone. So there is not that either/or connection described above, it just appeared that way at first.

I also just noticed, that the same that goes for the message list also goes for the currently active account: here there is not even a highlight on direct focus, and no dimmed highlight as well.

VOOM108 commented 1 year ago

I got it! It was Dark Reader extension. I was assuming, that it only affects the message content (which is why I have it installed). I found a setting, though, that makes it leave the theme alone and only darken the message content. Affecting the theme was on by default, as it seems.

Betterbird commented 1 year ago

OK, let's try a summary here:

  1. Missing highlight of selected row without focus: Solved, Dark Reader was the culprit
  2. Still investigating what the desired state is. I can't see individual persistence when switching folders and coming back, not even in version 102. However, there is https://bugzilla.mozilla.org/show_bug.cgi?id=1849960 which complains about the same thing.
  3. Yes, for a proper tree impression, the so-called "alignment column", in your case, the From, is also indented. Otherwise it would look like a mess. This comes from Issue #23. A thread pane of 300px width like in your picture is not viable.

Closing this ticket since we're not going to action any of this. We'll update the ticket wrt. to point 2 in due course. If there's a fix to the referenced bug, it will appear in BB early, or we might fix it ourselves. As was said, still figuring out what it right or wrong.

In general, mixing three issues into one ticket is not a good idea, and support issues should be taken up with our support and not being filed as GitHub tickets.

VOOM108 commented 1 year ago

I still have questions, comments:

ad 2) Even a switch to open or close all threads would be helpful, and if the choice can't be persistent, at least a setting to define the initial state. I would set the initial state to closed (as opposed to open now) and then only open the ones I am currently working with. The way it is now, it is hard to see/find even the latest emails if there are one or more long conversations at the top.

ad 3) It is clear to me that the From also needs to be indented. I did not mean to not indent anything.

What I am suggesting is making the overall size of the single indentation smaller. e.g. like I described above.

One aspect would be to remove the "replied state" icon (here in purple), as it is not needed in threaded view:

image

In threaded view I see the reply directly in the next indented line, so the information is redundant.

As a result, both the From and the Subject line would move to the right:

image

While it looks like a small difference in the single row, in total it would give the whole thread a better layout.

Of course, the 300px pane was supposed to exaggerate the issue. But still, even with ca. twice that size (my regular setting) a long thread goes over the limit too soon.

Betterbird commented 1 year ago

ad 2) Are you not aware of * and \ commands, also in the menu: image That is reasonably well persisted.

ad 3) The indentation per level follows TB exactly, 16px per level: https://searchfox.org/comm-central/rev/d7f41c84799258d2aeb2faa4fc5b5992d618a6a4/mail/themes/shared/mail/threadPane.css#384

Betterbird commented 1 year ago

OK, the expert got back to me: There was never any persisting of individual states, so "I believe to remember it used to remember the state of individual threads" doesn't see to be correct.

VOOM108 commented 1 year ago

Ok, then I remembered that wrongly.

About the * and \ - even now you tell me, I cannot find that in any menu. Where exactly is that? Also, if those are Shortkeys, it may be an issue that on the German keyboard they are not single keypresses like on a US keyboard.

I am just now trying to change the indentation and replied icon via CSS. I am sure it follows TB exactly, I just think that it may be something else that BB could improve.

Betterbird commented 1 year ago

Just type * or \ on any keyboard. It's on the "View > Threads" menu, last entries.

Yes, you could change the indentation via userChrome.css. You need to change

tr[is="thread-row"] .subject-line {
  margin-inline-start: calc(16px * var(--thread-level));

and BB's extra CSS:

/* Indent "alignment column" as well. */
tr[is="thread-row"][multiline="true"] td[alignmentcolumn="true"] {
  padding-left: calc(6px + 16px * var(--thread-level));
}
tr[is="thread-row"][multiline="true"] .correspondentcol-column {
  background-position-x: calc(4px + 16px * var(--thread-level));

Try changing the 16px to 12px and add ! important if it has no effect.

VOOM108 commented 1 year ago

Thanks for the hints. I still don't see this in any menu. I see no "Threads" under "View". I also have no numpad - can the hotkey bindings be changed? Otherwise, I'll try to remap it with AHK.

The CSS I am trying does not seem to be effective, even with !important. In the browser tools this looked like I wanted it:

tr[is="thread-row"][multiline="true"] td[alignmentcolumn="true"] {
  padding-left: calc(0px + 16px * var(--thread-level))!important;
}
tr[is="thread-row"][multiline="true"] .subject-line img {
  display:none!important; 
}

toolkit.legacyUserProfileCustomizations.stylesheets is set to true... I have only one profile and edtited the two userChrome.css in that profile (local and roaming)

What am I missing?

Betterbird commented 1 year ago

image

tr[is="thread-row"] .subject-line {
  margin-inline-start: calc(5px * var(--thread-level)) !important;
}

/* Indent "alignment column" as well. */
tr[is="thread-row"][multiline="true"] td[alignmentcolumn="true"] {
  padding-left: calc(6px + 5px * var(--thread-level)) !important;
}

works perfectly, notice the 5px indentation: image

Why does display:none make any sense? userChrome.css is in a directory in your profile under roaming.

VOOM108 commented 1 year ago

Ah, I see, it is only in the menu bar view menu (which I have hidden), not in the right side hamburger menu view menu. Strangely, I got the hotkeys to work with AltGr and Shift as well now. I will still rebind them to some single keys with AHK.

Thank you for your patience and help, I appreciate it!

The display:none is for the replied-icon image I want to remove (and the space if it is not shown with visibility:hidden).

I don't understand, why the CSS does not work for me, either. I added the CSS for the red labels in the message headers just to see I am in the right file - I am, that one works.

Betterbird commented 1 year ago

The display:none is for the replied-icon image I want to remove (and the space if it is not shown with visibility:hidden).

Don't do that, it will mess up the entire layout. Indentation between 5px and 10px is quite OK. Send your userChrome.css to our support and we'll take a look.

VOOM108 commented 1 year ago

I tried the display:none in the dev tools and it did exactly, what I wanted it to. But I will see how that goes, once the css is effective.

I have nothing in the userChrome.css otherwise, this is all there is:

/*

* Do not remove the @namespace line -- it's required for correct functioning
 */

@namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");

.message-header-label {
  color: red ! important;
  font-weight: bold ! important;
}

tr[is="thread-row"] .subject-line {
  margin-inline-start: calc(10px * var(--thread-level))!important;
}
tr[is="thread-row"][multiline="true"] .correspondentcol-column {
  background-position-x: calc(0px + 10px * var(--thread-level))!important;
}
tr[is="thread-row"][multiline="true"] td[alignmentcolumn="true"] {
  padding-left: calc(0px + 10px * var(--thread-level))!important;
}

tr[is="thread-row"][multiline="true"] .subject-line img {
  display:none!important; 
}

With the first entry being only a marker to see, if the userChrome.css works at all. With that element, I see the entry in the dev tools, the other don't seem to be interpreted at all.

Betterbird commented 1 year ago

Space before !

VOOM108 commented 1 year ago

I never did that in I don't know how many lines of CSS in my life; is that a specialty of CSS for TB?

Edit: changed it now, does not make a difference, though.

Betterbird commented 1 year ago

It was just a guess. Take out the namespace line, that wrecks everything.

EDIT: There is none present in https://www.betterbird.eu/expert-tips/userChrome-115.css.

VOOM108 commented 1 year ago

That was it... I had the css-files before updating already, so I suppose it always remained because it explicitly warns to not remove it.

THANK YOU :D

VOOM108 commented 1 year ago

Just in case so. is interested in the result of my changes, here is what I have settled with:

/* making indentations smaller */
tr[is="thread-row"] .subject-line {
  margin-inline-start: calc(5px * var(--thread-level)) ! important;
  opacity: 0.65;  /* subject slightly different color, works in both light and dark themes */
}
tr[is="thread-row"][multiline="true"] .correspondentcol-column {
  background-position-x: calc(4px + 5px * var(--thread-level)) ! important;
}
tr[is="thread-row"][multiline="true"] td[alignmentcolumn="true"] {
  padding-left: calc(4px + 5px * var(--thread-level)) ! important;
}
/* removing the replied-state icon and the space it occupies */
tr[is="thread-row"][multiline="true"] .subject-line img {
  display: none ! important; 
}
/* putting a border between single messages and conversations (to make up for the lesser indentations) */
table.tree-table {
  border-collapse: collapse ! important; /* This is needed to make the table rows show the border */
}
tr[is="thread-row"][multiline="true"].children,
tr[is="thread-row"][multiline="true"].singleton {
  border-top: 1px dotted rgba(137, 137, 137,0.5) ! important;  /* again this should work with dark and light themes */
}
/* the trashcan icon is too small since 115, making it visually equal size */
.tree-button-delete img {
  width: 17px;
}
/* Same with attachment icon, this is smaller, but visually fits */
:is(.attachmentcol-column img, .attachment-icon) {
  width: 15px;
}

This is the full content of userChrome.css ATM. I am quite content with the results... :)

userChrome-result

Betterbird commented 1 year ago

Thanks for the sample. Yes, the possibilities are endless. Maybe we should consider reducing the indentation to 10px in geneal.

What is the "initial" value of border-collapse? So borders are switched off? .singleton is a class only BB has. What is .children? (Yes, we should know, but we don't have all the code present all the time.) So who receives a border here? Singletons and rows which have children? We have some CSS :not(.children):not(.singleton) which is exactly the opposite.

Since this is in your userChrome.css which applies globally, table.tree-table will likely affect other places. We suggest to use table[is="tree-view-table"].

VOOM108 commented 1 year ago

The initial value is "separate" - I think because this creates a border on every TD (making double lines), a border on the TR is then ignored. I suspect that there are no table-borders anyway anywhere, so probably the global setting won't have any effect anywhere else. But more specificity is always better, so I will implement the suggestion.

.children seems to be a row that has children, as opposed to .singleton which does not. "not singleton" would include those rows that are children of .children. So the borders appear at the top of any single message that is not part of a conversation (like the first green one at the bottom) and a first row of a conversation. Thus making it easier to discern what belongs together.

That was not much of an issue before changing the indentation. But after that, while long conversations are still easily graspable, shorter ones were hard to discern from single message rows.

Betterbird commented 1 year ago

.children is a class that's added by upstream TB code. It's a bit of a misnomer since in other parts this is called "haschildren".

Anyway, we're done here.

Betterbird commented 1 month ago

In the context of Issue #160 I was playing around with reduced indentation. Turned out that if the correspondents column is used as alignment column, a bug was exposed, fixed here: https://github.com/Betterbird/thunderbird-patches/commit/cbe536592d29f96ef2814518cf6c2409f831a6cb#diff-c7394e272ceb2ec8d0985ce329601201572dcfb20034cf4b4d11d5cab84eb294R465

The custom CSS needs to be:

/* Reduce thread indentation */
tr[is="thread-row"] .subject-line {
  margin-inline-start: calc(5px * var(--thread-level)) !important;
}

tr[is="thread-row"][multiline="true"] .correspondentcol-column {
  background-position-x: calc(4px + 5px * var(--thread-level)) !important;
}

tr[is="thread-row"][multiline="true"] td[alignmentcolumn="true"].correspondentcol-column {
  padding-inline-start: calc(var(--tree-header-cell-padding) + 5px * var(--thread-level)) !important;
}

/* Indent "alignment column" as well. */
tr[is="thread-row"][multiline="true"] td[alignmentcolumn="true"] {
  padding-left: calc(6px + 5px * var(--thread-level)) !important;
}

Were you using Correspondents or From/Recipient?

VOOM108 commented 1 month ago

I use only from; further recipients I only see in the header above the message

Betterbird commented 1 month ago

OK, well, we reduced the indentation to 10px in the shipped product now. Before ran into the issue with the non-indented correspondents. That column is tricky and never aligns properly, but at least, now it is indented. The CSS I posted is correct now, we've also published it here: https://www.betterbird.eu/expert-tips/userChrome.css

You may want to update your userChrome.css just to avoid surprises later.