#2450 - Anchor tooltip to the element in 'Group Activity' area #2482
Conversation
group-income
|
||||||||||||||||||||||||||||
| Project |
group-income
|
| Branch Review |
sebin/task/#2450-anchored-closable-tooltip
|
| Run status |
|
| Run duration | 10m 25s |
| Commit |
|
| Committer | Sebin Song |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
10
|
Skipped
|
0
|
Passing
|
112
|
| View all changes introduced in this branch ↗︎ | |
SebinSong
changed the title
| } | ||
| if (!this.manual) { return false } | ||
| // Manually toggle on/off the tooltip (reference: ProfileCard.vue) | ||
| if (this.deactivated || !this.manual) { return } |
There was a problem hiding this comment.
Simplifying the existing logic here.
| @@ -67,6 +81,13 @@ export default ({ | |||
| type: String, | |||
| required: false, | |||
| default: '' | |||
| }, | |||
| anchorToElement: { | |||
There was a problem hiding this comment.
A prop to anchor the tooltip to the trigger element instead. Let me know if there is a suggestion for a better name.
| const { scrollX, scrollY } = window | ||
| const { width, height, left, top } = this.trigger | ||
| const windowHeight = window.innerHeight | ||
| const spacing = 16 | ||
| let x, y | ||
| let transform | ||
| let absPosition // For css top/left/bottom/right properties to use along with 'position: absolute' |
There was a problem hiding this comment.
In the case of anchoring the tooltip to a certain element (which is a trigger element in our case), Things are easier if css transform: translate(...) is used along with absolute positioning (specifying top, left, right, bottom properties). So added this variable here.
In my testing, on iOS there is no way for a mobile user to close this tooltip except by opening a different tooltip. Tapping on the background has no effect. I double-checked, and this still seems to be the case on |
I don't see this happening on my Android browsers and PWA, so I guess that is an iOS specific issue. |
|
@taoeffect Apart from the iPhone-specific issue mentioned above, I think this PR is ready. (I use Android device and cannot reproduce that issue on my machine) |
closes #2450
[Before fix]
tooltip-prev-fix.mp4
[After fix]
tooltip-after-fix.mp4
(cc. @taoeffect )
There are two issues pointed out in the issue which are,
1] Tooltip should be anchored to the app (rather than user's screen).
2] Tooltip should be closable.
Firstly, (As you can see in the video attached above too) I noticed 1. is not a mobile-specific issue but does happen in desktop too. Apparently, this behaviour was intentionally implemented by the component author as you can read in below comment too:
group-income/frontend/views/components/Tooltip.vue
Lines 169 to 173 in b6dc54d
The logic to determine tooltip position written as part of this custom-directive is currently widely used across the app. So as a fix for the issue, I added a component prop(
anchorToElement) that allows to opt out of this behaviour and attach the tooltip to the trigger element instead.@dotmacro Regarding 2. you pointed out, the tooltip does look unable to close on mobile-browser but it actually remains there because the trigger element stays focused once we tap on it on mobile. We can just unfocus it to make it disappear (it means not only clicking on other call-to-actions in the page but also merely clicking on a random position in the background too), which I think is right behaviour. So I haven't made any changes regarding 2.. Hope it sounds good.
Thanks,