Patch #31441

Show elements titles using jQuery UI tooltips

Added by Marius BALTEANU 7 months ago. Updated 3 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:UI
Target version:4.1.0

Description

Browser default title tooltip is quite non friendly for the user and/or UI development. For example, you cannot control the delay and HTML is not supported. Moving to jQuery UI Tooltip improves the current UI experience and will allow us in the future to make new improvements.

Regarding my proposed patch, I decided to not add an arrow to the tooltip because I don't think that it worth it.

date.png (23.1 KB) Marius BALTEANU, 2019-05-24 11:34

author.png (21.5 KB) Marius BALTEANU, 2019-05-24 11:34

tooltip.patch Magnifier (1.26 KB) Marius BALTEANU, 2019-05-24 11:38

tooltip_v2.patch Magnifier (1.17 KB) Marius BALTEANU, 2019-06-19 10:01

issue_subject_tooltip.png (50.7 KB) Marius BALTEANU, 2019-08-04 22:57

selectors_for_tooltip.patch Magnifier (1.76 KB) Marius BALTEANU, 2019-08-04 23:00

tooltip-for-issues@2x.png (11.1 KB) Go MAEDA, 2019-08-07 02:18

gantttooltip.png (47.7 KB) Antonio McDeal, 2019-09-06 16:52


Related issues

Related to Redmine - Feature #32029: Replace gantt and calendar tooltips with jquery tooltips New

Associated revisions

Revision 18260
Added by Go MAEDA 6 months ago

Show elements titles using jQuery UI tooltips (#31441).

Patch by Marius BALTEANU.

History

#1 Updated by Go MAEDA 7 months ago

  • Target version set to Candidate for next major release

+1 remarkably nice improvement!

#2 Updated by Go MAEDA 7 months ago

  • Target version changed from Candidate for next major release to 4.1.0

Setting the target version to 4.1.0.

#3 Updated by Mizuki ISHIKAWA 6 months ago

+1

#4 Updated by Seiei Miyagi 6 months ago

I found the XSS in attached patch.
If I fill the project description like below, then open http://localhost:3000/admin/projects and hover the title of the project, the alert is showing.

<script>alert(1)</script>

the content function needs to escape the $(this).prop('title')

#5 Updated by Marius BALTEANU 6 months ago

  • Assignee set to Marius BALTEANU

Seiei Miyagi wrote:

I found the XSS in attached patch.
If I fill the project description like below, then open http://localhost:3000/admin/projects and hover the title of the project, the alert is showing.

[...]

the content function needs to escape the $(this).prop('title')

You're right, thanks for pointing this out, I'll fix it.

#6 Updated by Marius BALTEANU 6 months ago

I think for now it's enough to disable the HTML support (attached patch).

What do you think, Seiei Miyagi?

#7 Updated by Seiei Miyagi 6 months ago

Thank you! It looks good to me.

Since the title attribute is too general and XSS may occur, I think it would be nice to disable HTML support.
IMO If you need HTML support, it may be better to add explicit attributes. (e.g. like html-safe? IDK)

#8 Updated by Go MAEDA 6 months ago

  • Status changed from New to Closed
  • Assignee changed from Marius BALTEANU to Go MAEDA

Thank you to everyone who involved in this patch.

#9 Updated by Marius BALTEANU 4 months ago

I've just observed some bad effects of my patch. For example, in the gantt chart, the tooltip is shown even if the issue subject is visible:

I propose to replace the current [title] selector (which is too generic) with:
- a custom attribute data-toggle="tooltip" for enable the tooltip on elements
- '.icon-only' selector to target all the links rendered only as icon

#10 Updated by Go MAEDA 4 months ago

Marius BALTEANU wrote:

I propose to replace the current [title] selector (which is too generic) with:
- a custom attribute data-toggle="tooltip" for enable the tooltip on elements
- '.icon-only' selector to target all the links rendered only as icon

Tooltip for issues or some other objects will disappear after applying the new patch. Please see the screenshot below for an example. It is a pity if I cannot see the useful information any longer.

#11 Updated by Marius BALTEANU 4 months ago

Go MAEDA wrote:

Tooltip for issues or some other objects will disappear after applying the new patch. Please see the screenshot below for an example. It is a pity if I cannot see the useful information any longer.

You're right, but the plan is to add to all other objects that should have tooltips one by one. I'll update the patch to include the tooltip for object links as well.

#12 Updated by Marius BALTEANU 4 months ago

Go Maeda, is it better if we remove the title the from gantt issue subject?

Mariuss-MacBook-Pro:redmine mariusbalteanu$ git diff
diff --git a/lib/redmine/helpers/gantt.rb b/lib/redmine/helpers/gantt.rb
index ef9475cea..ed2a8f8f7 100644
--- a/lib/redmine/helpers/gantt.rb
+++ b/lib/redmine/helpers/gantt.rb
@@ -747,7 +747,6 @@ module Redmine
         when Issue
           tag_options[:id] = "issue-#{object.id}" 
           tag_options[:class] = "issue-subject hascontextmenu" 
-          tag_options[:title] = object.subject
           children = object.children & project_issues(object.project)
           has_children = children.present? && (children.collect(&:fixed_version).uniq & [object.fixed_version]).present?
         when Version

#13 Updated by Go MAEDA 4 months ago

Marius BALTEANU wrote:

Go Maeda, is it better if we remove the title the from gantt issue subject?

I agree.

But I would like to suggest another option. How about setting "Assignee (Status)" instead of removing the subject? The assignee and status are important information but those are not prominent in gantt.

Marius, which do you prefer?

diff --git a/lib/redmine/helpers/gantt.rb b/lib/redmine/helpers/gantt.rb
index ef9475cea..cc5b97cd7 100644
--- a/lib/redmine/helpers/gantt.rb
+++ b/lib/redmine/helpers/gantt.rb
@@ -747,7 +747,7 @@ module Redmine
         when Issue
           tag_options[:id] = "issue-#{object.id}" 
           tag_options[:class] = "issue-subject hascontextmenu" 
-          tag_options[:title] = object.subject
+          tag_options[:title] = "#{object.assigned_to&.name} (#{object.status.name})".lstrip
           children = object.children & project_issues(object.project)
           has_children = children.present? && (children.collect(&:fixed_version).uniq & [object.fixed_version]).present?
         when Version

#14 Updated by Antonio McDeal 3 months ago

I think tool-tips like these should also be used as gantt tool-tips which display brief information when rolling cursor over issue lines of the timeline, because currently those kind of tool-tips in gantt aren't designed to spawn on the left, right, below or over the issue lines, depending on which side a free space available, so the tool-tip wouldn't end up half cut outside of the window.
I tested the way new tool-tips are displayed and saw that they actually support that kind of smart behavior.

#15 Updated by Go MAEDA 3 months ago

Antonio McDeal wrote:

I think tool-tips like these should also be used as gantt tool-tips which display brief information when rolling cursor over issue lines of the timeline, because currently those kind of tool-tips in gantt aren't designed to spawn on the left, right, below or over the issue lines, depending on which side a free space available, so the tool-tip wouldn't end up half cut outside of the window.
I tested the way new tool-tips are displayed and saw that they actually support that kind of smart behavior.

It is a big change to discuss on this issue. Could you open a new issue?

#16 Updated by Antonio McDeal 3 months ago

Go MAEDA wrote:

Could you open a new issue?

Of course, I will do now. Could you please add this ticket as related if a relationship would make sense? I don't have a patch at the moment, but if nobody minds I might try to compose something a tad bit later next week. So far I figured out that jQuery tool-tips should support bare HTML fed into the title fields of tags (I'm aware of possible XSS but there is a good workaround I should test). Then there is multiple things from old tool-tips to consider for cleanup, including in style sheets, which shalt not be overlooked.

#17 Updated by Go MAEDA 3 months ago

  • Related to Feature #32029: Replace gantt and calendar tooltips with jquery tooltips added

#18 Updated by Marius BALTEANU 3 months ago

  • Status changed from Reopened to Closed

Go MAEDA wrote:

Marius BALTEANU wrote:

Go Maeda, is it better if we remove the title the from gantt issue subject?

I agree.

But I would like to suggest another option. How about setting "Assignee (Status)" instead of removing the subject? The assignee and status are important information but those are not prominent in gantt.

Marius, which do you prefer?

I'll open a new issue to discuss this change (I should have done it from the beginning).

Also available in: Atom PDF