-
Notifications
You must be signed in to change notification settings - Fork 79
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
Setting opacity to 0 #18
Comments
@mrjjwright Interesting! I think this might be a bug. I'll keep you posted. 💪 |
Thanks, also it appears that strings like “0” work, which might be a clue.
… On Jul 22, 2017, at 9:48 PM, Jorge Bucaran ***@***.***> wrote:
@mrjjwright <https://github.com/mrjjwright> Interesting! I think this might be a bug. I'll keep you posted on this.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#18 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAQ9zKefLZao5iArQZHz-v_TYRqal3Aks5sQsKMgaJpZM4OgSVU>.
|
I think this line is why there is a bug ( |
Thanks @yepbug. Yes, that should do it. @mrjjwright Just curious. Why were you using a number instead of a string? |
Here's some interesting discussion: WICG/webcomponents#519, WICG/webcomponents#519 (comment) I think doing element.style[i] = value[i] != null ? value[i] || "" The most important thing would be to settle on the API first, should attributes accept only strings? how should other types be handled? And make it clear on the documentation. |
@acstll Why would you use a number instead of a string again? |
I can imagine getting the value out of a computation, for example, so you just use that. You do something like I'm in favor of only accepting strings as attribute values, so this here wouldn't be a bug. And I'm just giving my opinion here I'm not even using picodom myself :-) |
IMO, disallowing IMO, the challenges here are minimal - handling numbers poses no real difficulty, so there's no practical reason to create problems or surprises by disallowing numbers: principle of least astonishment. |
Sounds good, but we could argue that picodom shouldn't even bother with inline styles (allow setting style attribute only, which is currently impossible). Compared to CSS, inline styles are slow. Or one could use a CSS-in-JS solution. What do you think? |
IMO, support for inline styles is pretty universal across JSX implementations - it would suck not to have this, and (regardless of micro performance concerns) it already provides a basic CSS-in-JS solution, since you can simply create objects/modules with reusable CSS properties. If someone has extreme performance requirements, they can either use regular CSS or a CSS-in-JS solution, there are plenty of options, but basic object-to- |
I'm gonna PR this so we can discuss over a possible solution and not about things sucking or not. ✌️ |
Good call 😊 |
The PR was merged, so we can close this? |
I set opacity to 0 as follows:
h( "div", { class: "", style: { width: 15, height: 15, opacity: 0 }, oninsert: function(el) { checkEl = el; } }, "✔" )
Picodom is not outputting an opacity css style prop on the element. If I set it to 0.1 or higher it does.
The text was updated successfully, but these errors were encountered: