591535 - "Give me some help" link's iframe behavior is no longer necessary, on Bugzilla 4.0

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Max Kanat-Alexander]

As of now in 4.0, all of the displayed fields have their own help that you can see already in tooltips by hovering over them. So, the "Give me some help" link and the help.js system are no longer needed.

Comment 1

[Frédéric Buclin]

Before doing that, we should fix the query page to display the tooltip when hovering fields themselves, not only when hovering field labels.

Comment 2

[Max Kanat-Alexander]

Ah, it's true that the current system does that, but since we have the question-mark pointer that appears over the field label, I think the field label itself is enough. I don't think that showing the tooltip when hovering the field itself is actually something that users want--I know that for me, with the current help system, I find it rather annoying (particularly since it blocks out other parts of the UI).

Comment 3

[Max Kanat-Alexander]

Attachment: v1

Comment 4

[Frédéric Buclin]

(In reply to comment #2)
> Ah, it's true that the current system does that, but since we have the
> question-mark pointer that appears over the field label, I think the field
> label itself is enough.

But how often do you hover a label and see the pointer appearing as a question-mark? There is no hint anywhere mentioning that to get some help, the user should hover the field labels. I'm not saying this should block this bug, but that's something we could mention.

Comment 5

[Max Kanat-Alexander]

  Sure, I get what you're saying, but I don't really want to clutter up the page with anything else, if we can avoid it. The fact that the cursor changes is enough discoverability, I think, although pyrzak would be more of the expert in that area.

Comment 6

[Guy Pyrzak]

I agree that a lot of the code isn't necessary. However, I don't think (based on my own experience) that mouse overs/tooltips are super discoverable. I think we should keep the "give me some help" text and replace the reload with a popup or tooltip on click, which tells people to mouse over on the labels to get information about the fields they have questions about.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 472587 [details] [diff] [review]
v1

Okay, that makes sense. Perhaps we should move the link elsewhere, also?

Comment 8

[Guy Pyrzak]

we might be able to think hard and come up with one, but i think it's fine for now. It's not so bad that it should block the release IMO

Comment 9

[Max Kanat-Alexander]

(In reply to comment #8)
> we might be able to think hard and come up with one, but i think it's fine for
> now. It's not so bad that it should block the release IMO

  One what? (Was this comment on the wrong bug?)

Comment 10

[Guy Pyrzak]

Hrm, to clarify....

your question was: 

"perhaps we should move a link elsewhere, also?"

I responded with:

we might be able to think hard and come up with one (a new location for the link), but i think it's fine for
now. It's(the current location of the link) not so bad that it should block the release IMO

Comment 11

[Max Kanat-Alexander]

Ah, okay. :-)

Comment 12

[Guy Pyrzak]

Attachment: v2

feel free to just rewrite the help text itself. I'm not too happy with it.

Comment 13

[Max Kanat-Alexander]

Comment on attachment 481138 [details] [diff] [review]
v2

Well, I haven't looked this over in detail yet, but raw text can't be in .js files--it has to be in the templates, so that it can be localized.

Also, I really think we should go with something simpler. Why not just an alert or a hidden div?

Comment 14

[Guy Pyrzak]

I can do a hidden div. The code was already there so I just reused it.

Comment 15

[Max Kanat-Alexander]

(In reply to comment #14)
> I can do a hidden div. The code was already there so I just reused it.

  Yeah, I understand that. I was just thinking of this in the context of, "If we were newly-implementing something like this, would we want all that code complexity for it?" and I think that we wouldn't. Also, since it's going to be such a small feature going forward, I think that it should be as simple as possible. (Also, I'm always a fan of deleting code. :-) )

Comment 16

[Max Kanat-Alexander]

Hey hey. Any chance that we can get an updated patch for this bug? Soon it's going to be one of the last 4.0 blockers.

Comment 17

[Guy Pyrzak]

Attachment: v3

Comment 18

[Guy Pyrzak]

Attachment: v4

sorry i was dumb, should have used css instead of JS. I'm sure if i tried i could even do this all with just CSS. Let me know if that's what you'd prefer.

Comment 19

[Frédéric Buclin]

(In reply to comment #18)
> sorry i was dumb, should have used css instead of JS. I'm sure if i tried i
> could even do this all with just CSS. Let me know if that's what you'd prefer.

How? Using e.g. the :hover pseudo-class? If you can do it, that would be great, except that everybody would see the help being displayed.

Comment 20

[Frédéric Buclin]

Comment on attachment 486235 [details] [diff] [review]
v4

OK, I tested your patch, and I'm pretty sure this can be done with :hover only, or something equivalent. This means js/help.js could die entirely.

Your patch has two bugs anyway:
- Once you *click* on a label and come back, the JS code no longer works.
- When you click on the "Give me some help" link, the summary field flies to the right, moving outside the browser window.

Comment 21

[Guy Pyrzak]

The second bug I know about, the first bug, I'll need to know which browser you're using, it worked fine in chrome after clicking the label a bunch of times.

As far as only doing it with :hover only, i think you're correct, and I'll try that next. I just wanted to get forward progress on this.

Comment 22

[Guy Pyrzak]

Attachment: V5

css only version. I'm pretty happy with it

Comment 23

[Frédéric Buclin]

(In reply to comment #21)
> The second bug I know about, the first bug, I'll need to know which browser
> you're using

Firefox 3.6.11 on Linux.

Comment 24

[Guy Pyrzak]

doesn't matter now, it's all CSS so it should work on all browsers that support the :first-child and :hover psuedo-classes.

Comment 25

[Frédéric Buclin]

Comment on attachment 486357 [details] [diff] [review]
V5

Great patch! :) And works fine in Firefox, Google Chrome, Opera, Konqueror, Safari, IE8 and IE9. But it doesn't work in IE6. But I don't think we should prevent this patch from being used for a 10 years old browser. r=LpSolit

Comment 26

[Frédéric Buclin]

Maybe for IE6, we could display the div in place of the "Give me some help link". It's just a longer sentence, after all, but it won't break the UI.

Comment 27

[Max Kanat-Alexander]

Comment on attachment 486357 [details] [diff] [review]
V5

A div cannot be a child of a span.

Comment 28

[Max Kanat-Alexander]

Oh, and IE6 almost certainly does not support first-child.

Comment 29

[Max Kanat-Alexander]

Also, do you really want this to appear on hover, instead of on click? That means that the layout will move any time somebody accidentally moves their mouse over that link, right?

Comment 30

[Guy Pyrzak]

(In reply to comment #29)
I thought about div being a child of span, i'm happy to make it a div with display inline.

i can remove the first child thing and put in a class.

And it will show up whenever someone mouses over the text itself and only the text "give me some help". you can see a previous version that uses JS and a click. 

Basically you guys can pick what you prefer. I've shown both options and am indifferent. attachment 486232 [details] [diff] [review] uses simplified JS instead of CSS.

I could also combine them and use simpler JS (no xy stuff). Whatever you guys want works for me.

Comment 31

[Frédéric Buclin]

(In reply to comment #29)
> Also, do you really want this to appear on hover, instead of on click? That
> means that the layout will move any time somebody accidentally moves their
> mouse over that link, right?

No, the layout doesn't move when hovering the link.

Comment 32

[Frédéric Buclin]

We should consider the CSS-only approach. We really don't need JS for that at all.

Comment 33

[Frédéric Buclin]

mkanat, wouldn't things be easier with just a sentence of the form "Hover field labels to get help" in place of "Give me some help", and drop CSS/JS completely?

Comment 34

[Max Kanat-Alexander]

(In reply to comment #33)
> mkanat, wouldn't things be easier with just a sentence of the form "Hover field
> labels to get help" in place of "Give me some help", and drop CSS/JS
> completely?

  Sure, as long as it's unobtrusive, that sounds reasonable.

Comment 35

[Max Kanat-Alexander]

I'd go with the sentence, "Hover your mouse over the each field label to get help for that field."

Comment 36

[Guy Pyrzak]

Attachment: v6

Comment 37

[Frédéric Buclin]

Comment on attachment 489034 [details] [diff] [review]
v6

>=== modified file 'template/en/default/search/search-advanced.html.tmpl'

>+<p id="search_help">Hover your mouse over the each field label to get help for that field.</p>

Should this be "over each..." instead of "over the each..."? Otherwise looks good to me. r=LpSolit

Comment 38

[Max Kanat-Alexander]

Comment on attachment 489034 [details] [diff] [review]
v6

Looks good to me. :-)

Comment 39

[Max Kanat-Alexander]

And of course, yeah, remove the extra "the" on checkin, as LpSolit pointed out.

Comment 40

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
deleted js/help.js
deleted skins/standard/help.css
modified template/en/default/search/search-advanced.html.tmpl
Committed revision 7599.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
deleted js/help.js
deleted skins/standard/help.css
modified template/en/default/search/search-advanced.html.tmpl
Committed revision 7479.