Remove our textbox-search-icon rule after landing of M-C bug 1380268

RESOLVED FIXED in Thunderbird 56.0

Status

Thunderbird
Theme
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

unspecified
Thunderbird 56.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

11 months ago
Bug 1380268 moved the magnifying glass in front of the textbox. Together with our magnifying glass we have now two. best is to remove our.
(Assignee)

Comment 1

11 months ago
Created attachment 8887421 [details] [diff] [review]
textbox-search-icon.patch

Remove it and mirror the magnifying glass to be Linux/Windows system conform. macOS needs no change.

I also removed the mirroring for RTL mode as this was refused in bug 1374352 and it makes no sense to make this in our area but not in the toolkit areas.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8887421 - Flags: review?(jorgk)

Comment 2

11 months ago
Comment on attachment 8887421 [details] [diff] [review]
textbox-search-icon.patch

Review of attachment 8887421 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/themes/linux/mail/searchBox.css
@@ -8,5 @@
>    padding-top: 1px;
>  }
>  
> -.gloda-search-icon,
> -.textbox-search-icon {

Why do remove this?

Also, is this a typo? Missing "s"?
https://dxr.mozilla.org/comm-central/source/calendar/base/themes/common/widgets/calendar-widgets.css#69

https://dxr.mozilla.org/comm-central/search?q=textbox-search-icon&redirect=false
(Assignee)

Comment 3

11 months ago
Created attachment 8887583 [details]
searchbox.png

(In reply to Jorg K (GMT+2) from comment #2)
> Comment on attachment 8887421 [details] [diff] [review]
> textbox-search-icon.patch
> 
> Review of attachment 8887421 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mail/themes/linux/mail/searchBox.css
> @@ -8,5 @@
> >    padding-top: 1px;
> >  }
> >  
> > -.gloda-search-icon,
> > -.textbox-search-icon {
> 
> Why do remove this?

Because without removing, the textbox would have two magnifying glasses. See screenshot. Before the M-C change, on the left was no icon.

> Also, is this a typo? Missing "s"?

No. With the "s" it's the deck which contains the .textbox-search-icon and the .textbox-search-clear. With the M-C patch, the .textbox-search-icon is no more shown because of the icon on the left.

Comment 4

11 months ago
I've tried the patch now. The Gloda glass and the QF glass look quite different now when the looked the same before. I find that confusing. Can we not change one of them to be like the other?
(Assignee)

Comment 5

11 months ago
Yeah, should be doable.
(Assignee)

Comment 6

11 months ago
Created attachment 8887804 [details] [diff] [review]
textbox-search-icon.patch

Now with the same icon everywhere.
Attachment #8887421 - Attachment is obsolete: true
Attachment #8887421 - Flags: review?(jorgk)
Attachment #8887804 - Flags: review?(jorgk)

Comment 7

11 months ago
I think one of us is on a moving train. With this patch, the search box has two glasses, the Gloda glass on the left and the other darker and thicker one on the right. Maybe you attached the wrong patch.
(Assignee)

Comment 8

11 months ago
You are on M-C tip? Bug 1380268 needs to be applied to work correctly.
(Assignee)

Comment 9

11 months ago
Also actual Daily without my patch shows two glasses like my screenshot.

Comment 10

11 months ago
(In reply to Richard Marti (:Paenglab) from comment #8)
> You are on M-C tip?
No, but on the train ;-) I'll rebuild.

Comment 11

11 months ago
The glasses look fine now, but the Gloda glass is on the right and the other search glass(es) (plural if you count event search in Lightning) are on the left. Can they not all be on the same side?
(Assignee)

Comment 12

11 months ago
Gloda search is a bit different as clicking the glass starts the search (you remember the issue we had with hovering it when no text is in the box?). The other searchboxes show a delete button when something is entered in the box.

I can try for Gloda to move the glass to the left and show an arrow to start the search, but then after returning from Gloda tab the arrow will be shown also with empty box. Or I change Gloda to behave like the other boxes. What do you think?

Comment 13

11 months ago
Yes, I remember the issue.

I don't think we want to expose the Gloda problem we discussed, that empty boxes also have an active glass, by showing an active arrow additionally.

I already tried to move the glass to the right, but that seems a little hard since it sits in the textbox. Maybe CSS can do it. I think that would be sufficient. However, ideally Gloda would behave like the other boxes.

Comment 14

11 months ago
Thinking about this a bit more, the two boxes have different functionality:

QF: As soon as you type, the filter starts, hence the (x) to clear it.
Gloda: Typing into the field triggers an auto-complete, which you can accept/click or you can click the glass.

I don't think you can unify that behaviour. So I'd just move the Gloda glass to the left and be done with it. Or do you think it's bad UX to have

|(/°) I'm looking for this| and then you have to click on the glass (/°) before the search string, hmm.
(Assignee)

Comment 15

11 months ago
Created attachment 8887897 [details] [diff] [review]
textbox-search-icon.patch

This moves the glass in Gloda box to the left like macOS has already. With this patch I don't remove any XZL/JS logic which would be still available when needed.
Attachment #8887804 - Attachment is obsolete: true
Attachment #8887804 - Flags: review?(jorgk)
Attachment #8887897 - Flags: review?(jorgk)

Comment 16

11 months ago
Looks nice, but
1) Clicking the glass does nothing now
2) It lost the hover feedback.
Or am I doing something wrong?
(Assignee)

Comment 17

11 months ago
It's only a icon without functionality now. It makes not much sense to have a execute icon in front of the expression. The other searchboxes do also nothing with this icon.

Comment 18

11 months ago
Comment on attachment 8887897 [details] [diff] [review]
textbox-search-icon.patch

So people have to know to hit <enter> now, OK.
I'll get that landed so we don't have two glasses in Daily.
Attachment #8887897 - Flags: review?(jorgk) → review+
(Assignee)

Updated

11 months ago
Keywords: checkin-needed
(Assignee)

Comment 19

11 months ago
...When people have discovered this glass had a function. It only showed a different cursor and no other feedback (until bug 1373923).

Comment 20

11 months ago
https://hg.mozilla.org/comm-central/rev/7a2ed081a9cb7e8fbb653d8c051951a34afa2649
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 56.0
You need to log in before you can comment on or make changes to this bug.