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

All my previous pull requests, plus some bits #5

Closed
wants to merge 9 commits into from

Conversation

mx-moth
Copy link
Contributor

@mx-moth mx-moth commented Nov 28, 2011

All the individual pull requests I sent (#2, #3, #4) are stand alone chunks of work, but here is a branch containing all of them, plus some extra clean up that does not deserve a pull request of its own. This is how I am using this app in the application that I am currently building, and is how I envisioned my changes to ultimately be applied.

- Turned the JavaScript code in to a jQuery plugin, removed most of the
  initialization code from the individual widget templates to a external
  JavaScript file, and added a {% phileo_js %} tag to load this plugin.
- Each like button gets a unique ID, so multiple like buttons can appear
  on a single page
- Made the widget a plain old form. This way, it works without
  JavaScript. Additionally, the CSRF stuff is included in the form by
  default, so the ajax_csrf.js script is obsolete
@paltman
Copy link
Member

paltman commented Nov 28, 2011

Tim,

First let me say, THANKS! I appreciate all the pull requests and explanations. I will be reviewing them and merging them for a target of a new release ASAP.

Patrick

The widget_id when rendering a like widget defaulted to None, which
broke the widget when no id was supplied. This creates a default id for
widgets, which should be unique for that model instance.
{% likes user as like_list %} defaults to using all registered models
when no models are specified, to allow easy listing of all likes in an
application - which is probably the main use case for this.
{% render_like like %} renders the given like and prints it in the
template. The template that it uses depends upon the liked instance, and
is either:

* phileo/<app>/<model>.html
* phileo/<app>/like.html
* phileo/like.html

This allows for custom like templates on a per model and per application
basis.
@paltman
Copy link
Member

paltman commented Nov 28, 2011

I really disagree with the "Remove all the CSS cruft..." commit. It's not wrong to ship with some css that just works out of the box. It's not dictating anything as you can just not include the css by omitting the tag and style it yourself as a site developer.

@paltman
Copy link
Member

paltman commented Nov 28, 2011

Can you update your fork with the master from this repo and create two new pull requests with the last two commits you have there (like list tag -- will need to be reworked based on the settings change I made where I use setting instead of registry, and the render_like tag)? Thanks.

@paltman
Copy link
Member

paltman commented Nov 28, 2011

I am still very interested in 1193aaa and 5a41e83 but think it best if they are updated with what's now currently on master and sent as new pull requests, if that isn't too much trouble. Therefore I am closing this pull request. Thanks!

@paltman paltman closed this Nov 28, 2011
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.

None yet

3 participants