Skip to content

Impact add tooltip for nodes#17450

Open
tsmr wants to merge 30 commits intoglpi-project:mainfrom
tsmr:ImpactAddTooltip
Open

Impact add tooltip for nodes#17450
tsmr wants to merge 30 commits intoglpi-project:mainfrom
tsmr:ImpactAddTooltip

Conversation

@tsmr
Copy link
Collaborator

@tsmr tsmr commented Jul 4, 2024

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #number

Impact add tooltip for nodes :

image

@cconard96 cconard96 self-requested a review July 5, 2024 10:20
Co-authored-by: Curtis Conard <cconard96@gmail.com>
tsmr and others added 4 commits July 9, 2024 12:05
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Copy link
Contributor

@AdrienClairembault AdrienClairembault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1) The tooltip is partially hidden by other items:

image

I would expect it to be displayed over them, what do you think ?

2) The tooltip content is built using tables.
GLPI used to overuse tables, we have mostly corrected that now and are thus avoiding tables when displaying non-tabular data.

3) IMO the tooltip is displayed too fast.
I don't want to see it every time I move my mouse over an item, or when I am doing others actions like drawing edges or moving nodes.

There should be something like a 0.3 to 0.5s delay before the content is displayed (this seems to be the standard for this kind of UX delay).
Maybe also something to prevent the tooltip from triggered when moving nodes ?

4) As stated before the content should come from an AJAX request IMO.
This is better for performances, the new code will load a lot of individual objects one by one which may be bad for big graphes.
It also allow to generate the content using twig templates instead of raw DOM computation from javascript.

Add function for tooptip
Add menu for see tooptip in getContextMenuItems
Add function for cleanPopper on grab & load another tooltip
@tsmr
Copy link
Collaborator Author

tsmr commented Aug 19, 2024

@AdrienClairembault

I do commit

Change z-index
Add function for tooptip
Add menu for see tooptip in getContextMenuItems
Add function for cleanPopper on grab & load another tooltip

Can you check ?

@AdrienClairembault
Copy link
Contributor

AdrienClairembault commented Aug 19, 2024

With the latest changes I don't have any tooltips, there seems to be an issue somewhere.

@tsmr
Copy link
Collaborator Author

tsmr commented Aug 20, 2024

With the latest changes I don't have any tooltips, there seems to be an issue somewhere.

When you click on informations menu, you must move your mouse on the icon object to see tooltip

Copy link
Contributor

@AdrienClairembault AdrienClairembault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I am not sold on the latest change that require to use the context menu to "enable" the tooltips of an item.

impact

In my opinion, I find it not very intuitive.

@tsmr
Copy link
Collaborator Author

tsmr commented Oct 4, 2024

Ok but what is the best approach so ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants