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

Please stop hardcoding styles & animations #26

Closed
joshribakoff opened this issue May 9, 2014 · 2 comments
Closed

Please stop hardcoding styles & animations #26

joshribakoff opened this issue May 9, 2014 · 2 comments

Comments

@joshribakoff
Copy link

Instead of using jQuery's "fadeIn" & "fadeOut" you should instead consider applying a semantic class. As a developer I can hook CSS transitions to these semantic classes if I do want to opt-in to animations.

For example:

function onEntryMouseEnterForCaption (sender) {
            $(sender.currentTarget).find('.caption').stop().fadeTo(500, 0.7);
        }

I wish that would change to:

function onEntryMouseEnterForCaption (sender) {
            $(sender.currentTarget).find('.caption').toggleClass('mouse-over');
        }

I can then choose what opacity I want to correspond with the mouse being over it. 0.7 is an arbitrary opacity that you've chosen because you like it, but its qualitative & other people may not like it at 0.7 but may want 1.0... But this is just a simple example. Maybe I don't even want to animate the opacity but maybe I instead want to 'slide' in with CSS transform, maybe I want a custom bezier acceleration curve with keyframes, etc..

Another example of why its problematic is you fadeOut() for 500ms then fadeIn() for another 500ms. I'm screen capturing the page with phantomjs to generate a thumbnail for the gallery itself, and this adds an additional 1000ms latency before I can screen capture the page... so I'm now having to fork the project simply because I want to opt out of this 1000ms animation that's being animated for a computer who doesn't even see it (headless web browser client).

Please consider allowing us to disable the animations via a setting, or even better, simply toggle classes like switch the elements class from jg-loading to jq-loaded and let us users apply the styles that we want for those classes.

Putting the animations into JS makes things impossible to override. Also jquery animations can kill performance on mobile devices whereas CSS3 transitions are very smooth on all devices.

@miromannino
Copy link
Owner

Yes as developer I think the same things, but one needs to consider people that don't want or don't know how to use these classes. These settings, and in particular the opacity to 0.7 (very bad), has been done only to work with IE. If you want, one can do it's own branch with your changes if you don't want to support older IEs.

If will do extra options for the animations and the opacity settings. So it can be useful.

@joshribakoff
Copy link
Author

I urge you to reconsider. There is no need for a user to understand. The extension already uses a css file. You provide default animations there. Jquery animations are going the way of html tables. I probably will fork it if you keep it as is, its perfect in every way, except the hard coded animations make it completely unusable for me..

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

No branches or pull requests

2 participants