Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MBS-12459: Mimic native tooltip position offset #2567

Merged
merged 1 commit into from
Jun 29, 2022
Merged

MBS-12459: Mimic native tooltip position offset #2567

merged 1 commit into from
Jun 29, 2022

Conversation

jesus2099
Copy link
Contributor

@jesus2099 jesus2099 commented Jun 14, 2022

Problem

Timeline custom tooltips are very nice but they are always partially covered by my mouse cursor.
I have noticed that the timeline custom tooltips were closer to the mouse than were the native tooltips (<*** title="native tooltip">).

See MBS-12459 for before/after photos on Windows 10.

Solution

I changed the timeline.js custom tooltip position offset to match the position, relative to mouse cursor, of the native tooltip.

Checklist for review

  • Test on Windows 10
  • Test on Linux
  • Test on Mac (if anyone can do this)
  • Test on mobile (maybe custom tooltip will look too far away from the triggering line) (no native tooltip on mobile)

@jesus2099 jesus2099 changed the title MBS-12459. Mimic native tooltip position offset MBS-12459: Mimic native tooltip position offset Jun 14, 2022
@jesus2099
Copy link
Contributor Author

Wow, @yvanzo! You have tested the change on Linux, already? :)
Was it bad before? and better after?

Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

Yes, it is slightly better than before as the cursor was sometimes (not always, it depends on when the event was triggered) hiding the beginning of the bubble.

Tested locally on Linux but I don’t think there will be any issue with desktop platforms anyway. It is more about dealing with smaller resolutions: tablets and mobiles.

I pushed this commit to the test branch and deployed it in the hope that everyone could test it with their own device from https://test.musicbrainz.org/statistics/timeline/main. Unfortunately, I couldn’t load the statistics data in the test database for now, so you will have to wait a bit.

@jesus2099
Copy link
Contributor Author

jesus2099 commented Jun 16, 2022

Hi @yvanzo,
Now I see that you managed to load some data now in TEST so we can compare with MBS side by side!!
Thank you very much! 👍 👍 👍

@jesus2099
Copy link
Contributor Author

jesus2099 commented Jun 16, 2022

On mobile, or more precisely, when there is no mouse cursor, it was clearly looking better before my change:

Arrow

IMG_20220616_161905
Before

IMG_20220616_161825
After

Hand

Here is a little bit special because you already cannot really use this feature conveniently.
As soon as you click a milestone (red vertical line), a new tab opens to show you the related detailed webpage.
It's when you close this info tab, that you can see the tooltip.

IMG_20220616_161922
Before

IMG_20220616_161945
After

@yvanzo
Copy link
Contributor

yvanzo commented Jun 29, 2022

On mobile, or more precisely, when there is no mouse cursor, it was clearly looking better before my change

This page is clearly not supporting mobile at the moment; See the overlapping date labels on the horizontal axis for example.

Being able to read the first letters of the tooltips on desktop seems to overcome the slightly increased distance on mobile.

@yvanzo yvanzo merged commit 8252aa6 into metabrainz:master Jun 29, 2022
@yvanzo
Copy link
Contributor

yvanzo commented Jun 29, 2022

Tests really are about desktop/mobile thus skipped testing on mac as it has already been tested on two desktop platforms.

reosarevok added a commit that referenced this pull request Jun 29, 2022
* master:
  Update POT files using the production database
  Update translations from Transifex
  MBS-12459. Mimic native tooltip position offset (#2567)
  MBS-12472: Show genre edit history links for all users (#2574)
  Fix warning when displaying blank annotation text (#2578)
  Improve SQL formatting for language queries
  Add stat label for null language works
  MBS-12470: Fix work language query for null
  MBS-12470: Also list language = null in the stats page
  MBS-12470: Fix release language query for null
  Remove misleadingly-placed TODO
  MBS-12479: Add aliases for genres to the website
  MBS-12464: Also display event-work rels in Related Works (#2573)
  Remove seemingly useless test sql files (#2571)
  MBS-12474: Display Wikipedia extracts for genres (#2576)
  MBS-12475: Add autoselect for RYM genre URLs
  MBS-12475: Add autoselect for Wikidata genre URLs
@jesus2099
Copy link
Contributor Author

Thank you!

reosarevok added a commit that referenced this pull request Jul 3, 2022
* beta:
  Update POT files using the production database
  Update translations from Transifex
  MBS-12484: Hide genre alias edit links if limited (#2581)
  Update POT files using the production database
  Update translations from Transifex
  MBS-12459. Mimic native tooltip position offset (#2567)
  MBS-12472: Show genre edit history links for all users (#2574)
  Fix warning when displaying blank annotation text (#2578)
  Improve SQL formatting for language queries
  Add stat label for null language works
  MBS-12470: Fix work language query for null
  MBS-12470: Also list language = null in the stats page
  MBS-12470: Fix release language query for null
  Remove misleadingly-placed TODO
  MBS-12479: Add aliases for genres to the website
  MBS-12464: Also display event-work rels in Related Works (#2573)
  Remove seemingly useless test sql files (#2571)
  MBS-12474: Display Wikipedia extracts for genres (#2576)
  MBS-12475: Add autoselect for RYM genre URLs
  MBS-12475: Add autoselect for Wikidata genre URLs
  Update POT files using the production database
  Update translations from Transifex
  Rewrite "Test X" test names (#2575)
  Bump Flow to 0.181.1
  MBS-12458: Error if trying to load annotation for wrong entity (#2568)
  MBS-11599: Allow adding URL relationships to genre pages (#2497)
  Fix the hydrate div name for AliasEditForm (#2490)
  Extend useful Controller::Ratings tests to other entities
  Document and standardize Controller::Tags tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants