Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Improve tooltip positioning #8219

Merged
merged 1 commit into from
May 12, 2022

Conversation

weeman1337
Copy link
Contributor

@weeman1337 weeman1337 commented Apr 3, 2022

I actually wanted to fix the tooltip of the „decryption issue“ message.
After spending some time on this I found that tooltips can be improved in other places as well.

This PR partially refactors tooltips. Worth mentioning:

  • Left/right tooltips are now vertically centre aligned to their parent
  • Removed yOffset

❗Notes

  • I've tested all tooltips I can find. If you know a hidden or special one just leave a comment.
  • Like mentioned above the left/right tooltips changed (top vs centre aligned).
    This is just a suggestion. But so far in my opinion this improves the tooltips.

The changes are improving the tooltip positioning in many places, for example:

Where Before After
call buttons image
(tooltip appears above another button)
image
search button image
(unaligned)
image
„decryption issue“ image
(cut off)
image
spaces buttons image
(tooltip appears above another button)
image

Notes: Tooltip positioning has been improved


Here's what your changelog entry will look like:

🐛 Bug Fixes

Preview: https://pr8219--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@weeman1337 weeman1337 force-pushed the feature-refactor-tooltips branch 3 times, most recently from 5aa74cf to 6d44bc6 Compare April 3, 2022 21:50
@codecov
Copy link

codecov bot commented Apr 3, 2022

Codecov Report

Merging #8219 (6d44bc6) into develop (3e31fdb) will decrease coverage by 2.17%.
The diff coverage is 27.77%.

❗ Current head 6d44bc6 differs from pull request most recent head 7837b66. Consider uploading reports for the commit 7837b66 to get more accurate results

@@             Coverage Diff             @@
##           develop    #8219      +/-   ##
===========================================
- Coverage    30.85%   28.67%   -2.18%     
===========================================
  Files          893      851      -42     
  Lines        50793    49774    -1019     
  Branches     12928    12655     -273     
===========================================
- Hits         15671    14274    -1397     
- Misses       35122    35500     +378     
Impacted Files Coverage Δ
src/components/structures/SpaceHierarchy.tsx 3.31% <ø> (ø)
src/components/views/beta/BetaCard.tsx 12.12% <ø> (-7.33%) ⬇️
src/components/views/dialogs/ForwardDialog.tsx 86.76% <ø> (ø)
...ponents/views/elements/AccessibleTooltipButton.tsx 66.66% <ø> (ø)
src/components/views/elements/FacePile.tsx 10.00% <ø> (-67.78%) ⬇️
src/components/views/elements/Pill.js 61.60% <0.00%> (+0.34%) ⬆️
src/components/views/elements/TooltipTarget.tsx 100.00% <ø> (ø)
...c/components/views/right_panel/RoomSummaryCard.tsx 15.38% <ø> (ø)
src/components/views/rooms/EventTile.tsx 46.60% <0.00%> (+0.15%) ⬆️
src/components/views/rooms/MessageComposer.tsx 49.09% <ø> (ø)
... and 486 more

@robintown robintown added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Apr 4, 2022
@weeman1337 weeman1337 marked this pull request as ready for review April 5, 2022 14:39
@weeman1337 weeman1337 requested a review from a team as a code owner April 5, 2022 14:39
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

I think this requires design review before code review is a reasonable course of action, to verify the tooltip behaviour and expectations for at least the sample cases in the PR description.

@turt2live turt2live requested a review from a team April 9, 2022 05:17
@MadLittleMods MadLittleMods added the Z-Community-PR Issue is solved by a community member's PR label Apr 29, 2022
Copy link

@gaelledel gaelledel left a comment

Choose a reason for hiding this comment

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

lgtm!

@t3chguy t3chguy requested a review from turt2live May 6, 2022 15:36
@turt2live turt2live requested review from a team and removed request for turt2live May 7, 2022 01:35
@t3chguy
Copy link
Member

t3chguy commented May 9, 2022

@weeman1337 can you fix the merge conflicts so that it can be reviewed, thanks!

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

No questions otherwise, this is a great improvement!

Signed-off-by: Michael Weimann <michaelw@matrix.org>
Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

Looks good now, but for future reference, please avoid force pushing after review since it makes it more difficult to determine what's changed

@weeman1337 weeman1337 merged commit 7ed3089 into matrix-org:develop May 12, 2022
@weeman1337 weeman1337 deleted the feature-refactor-tooltips branch May 12, 2022 12:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants