-
Notifications
You must be signed in to change notification settings - Fork 40
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
speed up pico #24
Comments
I would support this change. Pull requests are certainly welcome. I'll need to consider whether this would require a major version bump, though. I'm thinking "yes", since it would be a significant change in behavior. This also makes me think that an example using css animations should probably be added to the README. |
i've made an example by reseting the default style that comes with pico i believe that css should be shipped separate, maybe in examples, in order to keep pico as light as possible and easy to compose on it. still, i found a bug regarding |
I think "very expensive" is rather subjective -- I've certainly never had any performance concerns with picoModal in my projects. Consider the possibility that this might be a micro-optimization with more drawbacks than benefits... because the At the very least if you decide to go down this route, please be aware of accessibility concerns... simply using transform and/or opacity to show/hide the modal might not actually make the content "disappear" to screen readers -- you'll probably want to utilize the "aria-hidden" attribute as well. |
hey,
i noticed that the way pico shows/hides the modal is via
display:none/block
property.this method is very expensive since it involves DOM redraw.
i've read how-to-achieve-60-fps-animations-with-css3
and would be great to switch to
transform
andopacity
way of doing things.The text was updated successfully, but these errors were encountered: