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

fixed tooltip position on mobile #2436

Merged
merged 3 commits into from
Dec 1, 2024
Merged

Conversation

Vayras
Copy link
Collaborator

@Vayras Vayras commented Nov 27, 2024

issue: #2434

fixed the tooltip position on mobile
Screenshot 2024-11-27 at 11 52 08 AM

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Awesome - thanks for your first PR @Vayras!

I found a bug - please be sure to thoroughly test all PRs and check the console for errors, in this case there's an issue with bottom-center not being a valid Tooltip direction:

Screenshot 2024-11-27 at 12 37 28 PM

I recommend also testing with dummy data, you can temporarily modify GroupMembersActivity.vue to use dummy data to test the display.

@Vayras
Copy link
Collaborator Author

Vayras commented Nov 28, 2024

@taoeffect thanks for the review I will fix this 🚀

@Vayras
Copy link
Collaborator Author

Vayras commented Nov 30, 2024

@taoeffect I have fixed the direction property can you please review this again for me? Thanks

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

This change has the same problem.

@corrideat
Copy link
Member

@Vayras If you take a look at Tooltip.vue, these are the allowed attributes:

    direction: {
      type: String,
      validator: (value) => ['bottom', 'bottom-left', 'bottom-right', 'right', 'left', 'top', 'top-left'].includes(value),
      default: 'bottom'
    },

If you need other values, you need to modify Tooltip.vue. However, your PR 'works' because the default value of bottom will actually be centred at the bottom.

@Vayras
Copy link
Collaborator Author

Vayras commented Dec 1, 2024

@corrideat thanks for the info I looked at the file and found an attribute "bottom-middle" which is as you said the default attribute as well so I'm using that
Screenshot 2024-12-02 at 12 07 43 AM

@taoeffect
Copy link
Member

@Vayras What file are you seeing that in? I don't see that in any file in the project.

@Vayras
Copy link
Collaborator Author

Vayras commented Dec 1, 2024

@corrideat Thanks for the heads up I think I was referring to the wrong tooltip file, fixed that in the latest commit

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Seems to work in my tests, great job @Vayras!

@taoeffect taoeffect merged commit 5f68446 into okTurtles:master Dec 1, 2024
3 checks passed
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.

3 participants