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

Fix overflown colors hidden #26

Merged
merged 4 commits into from
May 27, 2024
Merged

Conversation

pkong-ds
Copy link
Collaborator

  • scrollable
  • tooltip won't hide

@pkong-ds pkong-ds merged commit 632e64c into oursky:main May 27, 2024
1 check passed
Comment on lines +368 to +374
function handleColorPickersTooltip() {
tippy("[data-tippy-content]", {
placement: "bottom-end",
theme: "light-border",
});
}

Copy link
Member

Choose a reason for hiding this comment

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

May I know how the original tooltips works?

And if we are using bootstrap, did we explore the bootstrap tooltips instead of loading in tippy.js.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

how the original tooltips works?

IIUC, there were no tooltip. I added it for better UX

if we are using bootstrap

I haven't used that in my styling, so went for the first google result which demo & API LGTM

did we explore the bootstrap tooltips instead of loading in tippy.js

Agree with this, maybe should've checked bootstrap first. Do you think we should remove tippy?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't used that in my styling,

Sure? May be remove it. If we can't remove it, did you check which version of bootstrap we are using? Is it easy to upgrade?

I will prefer to use bootstrap tooltips if it fit the purpose.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okok, lemme check check, I prefer removing bootstrap if nothing using it

prefer to use bootstrap tooltips

May I know why? Is it just because the project was using it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just checked, ListItem.astro is using. Will remove tippy & use bootstrap

Copy link
Member

Choose a reason for hiding this comment

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

May I know why? Is it just because the project was using it?

Yes and we can't randomly add lib/way of doing things to make the project become legacy project.

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