Patch #31441
closedShow elements titles using jQuery UI tooltips
Added by Marius BĂLTEANU over 5 years ago. Updated almost 5 years ago.
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.
Files
date.png (23.1 KB) date.png | Marius BĂLTEANU, 2019-05-24 11:34 | ||
author.png (21.5 KB) author.png | Marius BĂLTEANU, 2019-05-24 11:34 | ||
tooltip.patch (1.26 KB) tooltip.patch | Marius BĂLTEANU, 2019-05-24 11:38 | ||
tooltip_v2.patch (1.17 KB) tooltip_v2.patch | Marius BĂLTEANU, 2019-06-19 10:01 | ||
issue_subject_tooltip.png (50.7 KB) issue_subject_tooltip.png | Marius BĂLTEANU, 2019-08-04 22:57 | ||
selectors_for_tooltip.patch (1.76 KB) selectors_for_tooltip.patch | Marius BĂLTEANU, 2019-08-04 23:00 | ||
tooltip-for-issues@2x.png (11.1 KB) tooltip-for-issues@2x.png | Go MAEDA, 2019-08-07 02:18 | ||
gantttooltip.png (47.7 KB) gantttooltip.png | Anonymous, 2019-09-06 16:52 |
Related issues
Updated by Go MAEDA over 5 years ago
- Target version set to Candidate for next major release
+1 remarkably nice improvement!
Updated by Go MAEDA over 5 years ago
- Target version changed from Candidate for next major release to 4.1.0
Setting the target version to 4.1.0.
Updated by Seiei Miyagi over 5 years 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')
Updated by Marius BĂLTEANU over 5 years ago
- Assignee set to Marius BĂLTEANU
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.
Updated by Marius BĂLTEANU over 5 years ago
- File tooltip_v2.patch tooltip_v2.patch added
I think for now it's enough to disable the HTML support (attached patch).
What do you think, Seiei Miyagi?
Updated by Seiei Miyagi over 5 years 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)
Updated by Go MAEDA over 5 years ago
- Status changed from New to Closed
- Assignee changed from Marius BĂLTEANU to Go MAEDA
Thank you to everyone who involved in this patch.
Updated by Marius BĂLTEANU over 5 years ago
- File issue_subject_tooltip.png issue_subject_tooltip.png added
- File selectors_for_tooltip.patch selectors_for_tooltip.patch added
- Status changed from Closed to Reopened
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
Updated by Go MAEDA over 5 years ago
Marius BALTEANU wrote:
I propose to replace the current
[title]
selector (which is too generic) with:
- a custom attributedata-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.
Updated by Marius BĂLTEANU over 5 years 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.
Updated by Marius BĂLTEANU over 5 years 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
Updated by Go MAEDA over 5 years 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
Updated by Anonymous about 5 years ago
- File gantttooltip.png gantttooltip.png added
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.
Updated by Go MAEDA about 5 years 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?
Updated by Anonymous about 5 years 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.
Updated by Go MAEDA about 5 years ago
- Related to Feature #32029: Replace gantt and calendar tooltips with jquery tooltips added
Updated by Marius BĂLTEANU about 5 years 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).
Updated by Anonymous almost 5 years ago
When adding some (self made) plugins that use Highcharts graphs, etc., there is no "tooltip"
in the "title"
attribute.
So,
if ($('[title]').tooltip !== undefined) {
is necessary before calling the
tooltip()
function.Updated by Go MAEDA over 4 years ago
- Related to Defect #33250: Useless white box appears over the the tooltip in EdgeHTML added
Updated by Go MAEDA almost 4 years ago
- Related to Defect #34247: Web browser freezes when displaying workflow page with a large number of issue statuses added
Updated by Go MAEDA over 3 years ago
- Related to Defect #34834: Line breaks in the description of a custom field are ignored in a tooltip added