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

Make the JavaScript more extendable #36

Open
Zauberfisch opened this issue Mar 24, 2014 · 6 comments
Open

Make the JavaScript more extendable #36

Zauberfisch opened this issue Mar 24, 2014 · 6 comments

Comments

@Zauberfisch
Copy link
Contributor

Some parts of the Javascript are pretty much unextendable and unchangeable.
I am not sure if I am just to stupid to use entwine namespaces, but I have been trying for ages to overwrite the OrderForm submit handler.

the submit handler is bound using this.on('submit', function(e){}) in onmatch of $('.order-form')
We should refactor that to use the entwine onsubmit syntax.

@icecaster
Copy link
Contributor

While I agree that the implementation should be consistent and using the chosen way throughout -
I'm somewhat having the usual js debate between simple & dirty or complex & extendible.
Looking at the code for the order form, this be rewritten in a few lines of procedural jquery, making it easy to understand, without the need for entwine etc.
If greater flexibility was needed the file could be blocked and own handlers would be easy enough to create by adapting the code from the files...

What is your opinion on dirty vs extendible entwine?

@Zauberfisch
Copy link
Contributor Author

I care a great deal about extensibility, especially because I am in need of extensibility right now, and the order form is causing me a lot of pain.
Further more I also actually like entwine, I write my own code in entwine too, and since SilverStripe is using it, I think it would only make sense that SwipeStripe does so too.

@frankmullenger
Copy link
Contributor

It's been a while since I have looked at this. Not clear on entwine namespaces but I think you can use more specific selectors in some cases.

I'd like to continue using entwine, changing it out would probably mean altering some of the modifier modules e.g:
https://github.com/frankmullenger/silverstripe-swipestripe/blob/2.1/javascript/OrderForm.js#L21

dependency:
https://github.com/frankmullenger/silverstripe-swipestripe-flatfeetax/blob/master/javascript/FlatFeeTaxModifierField.js#L4

If you need to replace the javascript completely you could try replacing the OrderForm completely with Object::use_custom_class() and you can effectively replace the Javascript at the same time if that helps.

@thomasbnielsen
Copy link

If going with the entwine stuff - please use a lot of comments. That stuff is so geeky and very few frontenders know how it works.
Entwine might bring something to the table that is worth a lot, but please remember that for frontenders like me, it complicates things - more stuff to learn.

@frankmullenger
Copy link
Contributor

@NobrainerWeb fair point, given that SilverStripe uses entwine I think it is ok to keep using it personally but agree we want to keep it simple where possible.

@Zauberfisch
Copy link
Contributor Author

@frankmullenger well, yes, I could overwrite that class or block the file.
But if I need to completely block a file or overwrite a php class just to modify a javascript onsubmit behavior, then there is something wrong. especially because entwine is designed to be extendable.

@NobrainerWeb I actually find entwine to be a lot more comprehensible and clear than regular jQuery spaghetti code. But agreed, comments are always good.

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

4 participants