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

open/close tooltip controller #1501

Merged
merged 1 commit into from
Jan 25, 2023
Merged

open/close tooltip controller #1501

merged 1 commit into from
Jan 25, 2023

Conversation

524c
Copy link
Contributor

@524c 524c commented Jan 8, 2023

This PR is very useful to help with form validation.
It allows having a condition of not opening it if the value of data-tip is false. Please see this example I created to demonstrate its use.

https://svelte.dev/repl/79a08e408d9e464aad574b46fa9e1feb?version=3.55.0

@524c 524c changed the title condition to know whether or not to open the tooltip open/close tooltip controller Jan 8, 2023
@saadeghi
Copy link
Owner

saadeghi commented Jan 9, 2023

Thank you for the PR.
I think having the false value for a data attribute is not a good idea (actually having the word false as a string in the HTML). So what do you think about conditionally rendering the data-tip (Svelte syntax: data-tip={error || null}) and then having this CSS to hide .tooltips that have not data-tip:

.tooltip:not([data-tip]):hover:before,
.tooltip:not([data-tip]):hover:after {
  visibility: hidden;
  opacity: 0;
}

https://svelte.dev/repl/9f8ceaff8bf041068027373441972e1c?version=3.55.0

@524c
Copy link
Contributor Author

524c commented Jan 9, 2023

Thank you for the PR. I think having the false value for a data attribute is not a good idea (actually having the word false as a string in the HTML). So what do you think about conditionally rendering the data-tip (Svelte syntax: data-tip={error || null}) and then having this CSS to hide .tooltips that have not data-tip:

.tooltip:not([data-tip]):hover:before,
.tooltip:not([data-tip]):hover:after {
  visibility: hidden;
  opacity: 0;
}

https://svelte.dev/repl/9f8ceaff8bf041068027373441972e1c?version=3.55.0

Great, that works too 😁
It's just that in my complete code in typescript, the error control variable has the type string | boolean.

I'll make a new commit with your suggestion, thanks 👍

@524c
Copy link
Contributor Author

524c commented Jan 10, 2023

Thank you for the PR. I think having the false value for a data attribute is not a good idea (actually having the word false as a string in the HTML). So what do you think about conditionally rendering the data-tip (Svelte syntax: data-tip={error || null}) and then having this CSS to hide .tooltips that have not data-tip:

.tooltip:not([data-tip]):hover:before,
.tooltip:not([data-tip]):hover:after {
  visibility: hidden;
  opacity: 0;
}

https://svelte.dev/repl/9f8ceaff8bf041068027373441972e1c?version=3.55.0

Hey @saadeghi

I better understood what caused the false value to become "false" in my PR. It was the prettier I have in vscode, with an autosave option.

What I understood was the following:
"false" in the context of CSS is a quoted boolean.
"'false'" in the CSS context is a string.

At first glance we seem to think it's a string/word, but CSS treats it as a quoted boolean.

The Prettier adds quotes around the values. So I decided to check that he was right and that it complies with the CSS specification and how the value is handled. I created some variations around boolean values. If you can take a look, please:
https://svelte.dev/repl/8823dbfd36ed44e4a91aecb92ad9a178?version=3.55.0

With my original proposal we can use the Svelte syntax like this:
(Svelte syntax: data-tip={error})

Rather than:
(Svelte syntax: data-tip={error || null})

What do you think?

@524c
Copy link
Contributor Author

524c commented Jan 16, 2023

Thank you for the PR. I think having the false value for a data attribute is not a good idea (actually having the word false as a string in the HTML). So what do you think about conditionally rendering the data-tip (Svelte syntax: data-tip={error || null}) and then having this CSS to hide .tooltips that have not data-tip:

.tooltip:not([data-tip]):hover:before,
.tooltip:not([data-tip]):hover:after {
  visibility: hidden;
  opacity: 0;
}

https://svelte.dev/repl/9f8ceaff8bf041068027373441972e1c?version=3.55.0

Hey @saadeghi,

I was in doubt about which style was the best, but I realized some problems with my original proposal. I'm ready to make a new commit with your suggestion.

@saadeghi saadeghi merged commit 48a548d into saadeghi:master Jan 25, 2023
@saadeghi
Copy link
Owner

Thank you

inorganik pushed a commit to inorganik/daisyui that referenced this pull request Feb 7, 2023
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