feat: show tag hover card when hovering cursor on hashtag links #2621

shuuji3 · 2024-02-25T09:18:39Z

This implements the hovercard feature for hashtags.

resolve Show details on hover for mentions and tags in statuses #149
partially help the request for Highlight tags in post where you follow the tag #1833

I think there are several benefits of tag hovercard:

can follow/unfollow interested tags quickly
can check whether the tag has been followed in the past or not, without moving to each tag page. This is related to Highlight tags in post where you follow the tag #1833 and useful especially when the post has many hashtags.
can check how popular each tag is. Currently, clicking a hashtag link navigates users to the tag timeline (example: https://main.elk.zone/mastodon.social/tags/Fediverse) where they cannot see the history graph.

Screenshots

stackblitz · 2024-02-25T09:18:42Z

Run & review this pull request in StackBlitz Codeflow.

netlify · 2024-02-25T09:18:56Z

✅ Deploy Preview for elk-docs canceled.

Name	Link
🔨 Latest commit	`9b9d74b`
🔍 Latest deploy log	https://app.netlify.com/sites/elk-docs/deploys/65df7b40f1e01c000818ed5a

netlify · 2024-02-25T09:18:57Z

✅ Deploy Preview for elk-zone ready!

Name	Link
🔨 Latest commit	`9b9d74b`
🔍 Latest deploy log	https://app.netlify.com/sites/elk-zone/deploys/65df7b4020c8b00008922aa0
😎 Deploy Preview	https://deploy-preview-2621--elk-zone.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

patak-dev · 2024-02-25T09:58:54Z

This looks amazing! I agree the UX after this change for tags is greatly improved.

One thing before we merge, could you check the layout of that card?

I think we should have more gap between the text and the graph. And maybe push the star to the right of the tag name so the tag looks aligned with the text below.

userquin · 2024-02-25T10:02:28Z

components/account/TagHoverWrapper.vue

+  () => [props.tagName, targetIsVisible.value] satisfies WatcherType,
+  async ([newTagName, newVisible]) => {
+    if (newTagName) {
+      tag.value = await fetchTag(newTagName)


Ensure you dont fetch it if not in screen

the browser will hang

Good catch! The original implementation was fetching API too aggressively.😅

I moved the fetchTag() to TagCard component to ensure that it's only called when showing the card.

I'm going to take a look at this...

…rd` component

shuuji3 · 2024-02-25T11:21:31Z

@patak-dev I adjusted the layout a bit. What do you think?

shuuji3 · 2024-02-25T11:23:27Z

components/tag/TagCard.vue

@@ -32,13 +40,13 @@ function go(evt: MouseEvent | KeyboardEvent) {

 <template>
  <div
-    block p4 hover:bg-active flex justify-between cursor-pointer
+    block p4 hover:bg-active flex justify-between cursor-pointer flex-gap-2


This gap also improves the layout of the hashtag page in Explore tab for very long hashtag names.

I think for long tags the start should be top aligned

I'd like the tag to always be aligned with the text below. I proposed having the star to the right, but I see now that it isn't a good idea. Maybe we should have the star always at the top left, and then the tag and the text below to the right of it. Or the star could be at the right of the graph, kind of like the Follow button in the notification cards for people following you

OK, I tried another layout by making the follow button an independent block and placing it on the left. This ensures both texts are aligned and more robust with a long hashtag.

For Hovercard, items-center looks lined up better than the top. For Hashtag, both look fine.

@patak-dev Which position do you prefer if we use this layout?

Hovercard

align center (items-center)

align top (items-start)

Hashtag page

shuuji3 · 2024-02-25T11:30:31Z

Hmm, I'm not sure why snapshot tests start revealing <VMenu> tag here: https://github.com/elk-zone/elk/actions/runs/8037557151/job/21952513803#step:7:88

I wonder if it's safe to update the snapshot test. 🤔

userquin · 2024-02-25T11:33:23Z

Hmm, I'm not sure why snapshot tests start revealing <VMenu> tag here: https://github.com/elk-zone/elk/actions/runs/8037557151/job/21952513803#step:7:88

I wonder if it's safe to update the snapshot test. 🤔

We'll need to mock it, I'm going to fix it

userquin · 2024-02-25T11:37:59Z

@shuuji3 your local deps are stale, run nr test:ci in your local

Try updating main and update your branch, then pnpm install --frozen-lockfile && nr test:ci (should dedupe dependencies)

userquin · 2024-02-25T12:11:07Z

@shuuji3 the logic seems to be wrong, we should avoid fetching the tag until visible in the viewport, the browser will hang if lot of tags, the result is not deterministic since it depends on the current timeline content

shuuji3 · 2024-02-25T12:15:42Z

@userquin Does your local branch have this commit (381d166)? I observed that issue in the network tab on devtool, but that issue should be fixed after this commit.

userquin · 2024-02-25T12:18:29Z

yes, but this is wrong, the tagName will not prevent to fetch the tag once set, you have the fetch with toplevel await in the TagCard script setup

patak-dev · 2024-02-25T13:25:44Z

Thanks for playing with the design of the card, I think it looks a lot better now! 🧡

components/account/TagHoverWrapper.vue

This reverts commit de8e26a.

shuuji3 · 2024-02-25T14:12:11Z

Updated to the new layout (used items-center to be able to share the same template without the necessity of the layout prop) and fixed the remaining test error now.

composables/settings/definition.ts

patak-dev

Awesome!

I see it is still a draft, mark it as ready when you think we should merge it

shuuji3 · 2024-02-26T11:12:49Z

Ah, actually, the draft change was intentional (close/reopen was my mistake 😅). Thanks to userquin, the issue where multiple tag fetch caused the race condition and hanging up timeline was solved, and also duplicated calls fr the same tag are properly cached now.

However, it seems that the current TagCardWrapper still calls API requests to https://<server-domain>/api/v1/tags/<tag-name> when the user scrolls the timeline and sees a new tag for the first time.

This is problematic because this endpoint has 300 calls / 5 min. rate limit similar to other API endpoints so if the user scrolls in a short period and sees more than 300 tags, it will exceed the rate limit and the endpoint will start returning 429 (too many requests) during 5 min. window.

So I need to find a way not to call the tag endpoint until the user hovers the cursor on the tag link.

shuuji3 · 2024-02-27T16:01:50Z

Now, fetchTag() should be called only once when hovering it. Also, the component code can be simplified. I hope this works fine.

tag-hovering.mp4

components/account/TagHoverWrapper.vue

shuuji3 · 2024-02-27T16:13:07Z

I had to update the same test snapshot again for strange <VMenu>. I guess this original comment is correct in a way:
https://github.com/shuuji3/elk/blob/db070eed8111bfc71b0686f9003f8cd736aff55a/tests/nuxt/content-rich.test.ts#L163-L163

userquin · 2024-02-27T16:23:04Z

I had to update the same test snapshot again for strange <VMenu>. I guess this original comment is correct in a way

The bdi content should be there I'm going to check what's wrong in the content-render module

…ring

shuuji3 · 2024-02-27T16:36:39Z

TODO: show spinner before receiving the actual tag data.

userquin · 2024-02-27T18:11:15Z

Check #2632

shuuji3 · 2024-02-28T09:28:59Z

There was a good skeleton component for the tag: https://github.com/elk-zone/elk/blob/main/components/tag/TagCardSkeleton.vue

elk-taghover-skeleton.mp4

shuuji3 · 2024-02-28T09:36:55Z

f67be19 (#2621)

Also, there was a useful useElementHover utility (https://vueuse.org/core/useElementHover/) instead of the manual mouse event handling. This can be used in #2632 as well.

shuuji3 · 2024-02-28T10:08:14Z

OK, I think it's ready to merge.

elk-taghover-skeleton.mp4

patak-dev

Amazing work! 👏🏼

feat: show tag hover card when hovering cursor on hashtag links

385fa0f

text: update snapshot as TagHoverWrapper adds one <span> wrapper

0d3ec43

userquin reviewed Feb 25, 2024

View reviewed changes

shuuji3 added 2 commits February 25, 2024 19:44

fix: avoid aggressive tag fetch call by moving fetchTag() to `TagCa…

381d166

…rd` component

fix: adjust tag hovercard layout

32b1151

shuuji3 requested a review from userquin February 25, 2024 11:16

shuuji3 commented Feb 25, 2024

View reviewed changes

test: update vitest snapshot

de8e26a

userquin marked this pull request as draft February 25, 2024 12:09

chore: avoid fetching card until visible

f7c376a

userquin reviewed Feb 25, 2024

View reviewed changes

components/account/TagHoverWrapper.vue Outdated Show resolved Hide resolved

shuuji3 added 3 commits February 25, 2024 23:06

Revert "test: update vitest snapshot"

9b3184d

This reverts commit de8e26a.

fix: remove unnecessary async

1019773

feat: improve tag card layout

c1bce74

shuuji3 marked this pull request as ready for review February 25, 2024 14:12

shuuji3 requested review from patak-dev and userquin February 25, 2024 14:14

chore: cleanup tag hover wrapper

0f05142

patak-dev requested changes Feb 25, 2024

View reviewed changes

composables/settings/definition.ts Outdated Show resolved Hide resolved

feat: split hideAccountHoverCard and hideTagHoverCard settings

d19770d

shuuji3 closed this Feb 26, 2024

shuuji3 reopened this Feb 26, 2024

shuuji3 marked this pull request as draft February 26, 2024 10:48

patak-dev approved these changes Feb 26, 2024

View reviewed changes

feat: fetch tag cache just once after hovering

f6b2471

shuuji3 marked this pull request as ready for review February 27, 2024 16:01

test: update vitest snapshot

db070ee

userquin reviewed Feb 27, 2024

View reviewed changes

components/account/TagHoverWrapper.vue Outdated Show resolved Hide resolved

fix: check hovered value in watch and fetch tag cache each mouse hove…

8364abc

…ring

feat: show TagCardSkeleton during loading tag data

d327c60

refactor: use useElementHover for hover state detection

f67be19

fix: revert show delay to initial 500ms included by mistake

95fdea6

chore: update spanish entries

9b9d74b

patak-dev approved these changes Mar 4, 2024

View reviewed changes

patak-dev added this pull request to the merge queue Mar 4, 2024

Merged via the queue into elk-zone:main with commit e44833b Mar 4, 2024
13 checks passed

shuuji3 deleted the feat/tag-hover-card branch March 4, 2024 16:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: show tag hover card when hovering cursor on hashtag links #2621

feat: show tag hover card when hovering cursor on hashtag links #2621

shuuji3 commented Feb 25, 2024 •

edited

stackblitz bot commented Feb 25, 2024

netlify bot commented Feb 25, 2024 •

edited

netlify bot commented Feb 25, 2024 •

edited

patak-dev commented Feb 25, 2024

userquin Feb 25, 2024

userquin Feb 25, 2024

shuuji3 Feb 25, 2024

userquin Feb 25, 2024

shuuji3 commented Feb 25, 2024

shuuji3 Feb 25, 2024

userquin Feb 25, 2024

patak-dev Feb 25, 2024

shuuji3 Feb 25, 2024 •

edited

shuuji3 commented Feb 25, 2024

userquin commented Feb 25, 2024 •

edited

userquin commented Feb 25, 2024 •

edited

userquin commented Feb 25, 2024

shuuji3 commented Feb 25, 2024 •

edited

userquin commented Feb 25, 2024

patak-dev commented Feb 25, 2024

shuuji3 commented Feb 25, 2024 •

edited

patak-dev left a comment

shuuji3 commented Feb 26, 2024 •

edited

shuuji3 commented Feb 27, 2024

shuuji3 commented Feb 27, 2024

userquin commented Feb 27, 2024

shuuji3 commented Feb 27, 2024

userquin commented Feb 27, 2024

shuuji3 commented Feb 28, 2024

shuuji3 commented Feb 28, 2024 •

edited

shuuji3 commented Feb 28, 2024 •

edited

patak-dev left a comment

feat: show tag hover card when hovering cursor on hashtag links #2621

feat: show tag hover card when hovering cursor on hashtag links #2621

Conversation

shuuji3 commented Feb 25, 2024 • edited

Screenshots

stackblitz bot commented Feb 25, 2024

netlify bot commented Feb 25, 2024 • edited

✅ Deploy Preview for elk-docs canceled.

netlify bot commented Feb 25, 2024 • edited

✅ Deploy Preview for elk-zone ready!

patak-dev commented Feb 25, 2024

userquin Feb 25, 2024

Choose a reason for hiding this comment

userquin Feb 25, 2024

Choose a reason for hiding this comment

shuuji3 Feb 25, 2024

Choose a reason for hiding this comment

userquin Feb 25, 2024

Choose a reason for hiding this comment

shuuji3 commented Feb 25, 2024

shuuji3 Feb 25, 2024

Choose a reason for hiding this comment

userquin Feb 25, 2024

Choose a reason for hiding this comment

patak-dev Feb 25, 2024

Choose a reason for hiding this comment

shuuji3 Feb 25, 2024 • edited

Choose a reason for hiding this comment

Hovercard

align center (items-center)

align top (items-start)

Hashtag page

shuuji3 commented Feb 25, 2024

userquin commented Feb 25, 2024 • edited

userquin commented Feb 25, 2024 • edited

userquin commented Feb 25, 2024

shuuji3 commented Feb 25, 2024 • edited

userquin commented Feb 25, 2024

patak-dev commented Feb 25, 2024

shuuji3 commented Feb 25, 2024 • edited

patak-dev left a comment

Choose a reason for hiding this comment

shuuji3 commented Feb 26, 2024 • edited

shuuji3 commented Feb 27, 2024

shuuji3 commented Feb 27, 2024

userquin commented Feb 27, 2024

shuuji3 commented Feb 27, 2024

userquin commented Feb 27, 2024

shuuji3 commented Feb 28, 2024

shuuji3 commented Feb 28, 2024 • edited

shuuji3 commented Feb 28, 2024 • edited

patak-dev left a comment

Choose a reason for hiding this comment

shuuji3 commented Feb 25, 2024 •

edited

netlify bot commented Feb 25, 2024 •

edited

netlify bot commented Feb 25, 2024 •

edited

shuuji3 Feb 25, 2024 •

edited

align center (`items-center`)

align top (`items-start`)

userquin commented Feb 25, 2024 •

edited

userquin commented Feb 25, 2024 •

edited

shuuji3 commented Feb 25, 2024 •

edited

shuuji3 commented Feb 25, 2024 •

edited

shuuji3 commented Feb 26, 2024 •

edited

shuuji3 commented Feb 28, 2024 •

edited

shuuji3 commented Feb 28, 2024 •

edited