Hey Leslie,

The issues seem pretty simple so I will work and upload in the coming
two or three days (having some connection issues in my new "office") I
also finished the change so that all the javascript are in the
weblocks namespace.

Regards,

Mackram

On Aug 8, 5:56 pm, "Leslie P. Polzer" <[email protected]> wrote:
> I have finished reviewing the diff now. Remaining comments, mostly
> minor:
>
> @@ -605,8 +612,12 @@
>    "Returns a list of dependencies on scripts and/or stylesheets that
>     will persist throughout the whole application. See documentation
> for
>     'widget-application-dependencies' for more details."
> -  (build-local-dependencies
> -   (weblocks-webapp-application-dependencies app)))
> +  (append
> +   (if (weblocks-webapp-application-dependencies app)
> +       (build-local-dependencies
> +       (weblocks-webapp-application-dependencies app)))
> +   (if (weblocks-webapp-javascript-backend app)
> +       (build-client-backend (weblocks-webapp-javascript-backend
> app)))))
>
> These IF clauses don't have correct indentation. But since they are
> missing
> an else clause we can just use WHEN. And if you're not adverse to
> anaphoric
> macros (your choice) this boils down to
>
> (awhen (weblocks-webapp-application-dependencies app)
>   (build-local-dependencies it))
>
> + (defun build-client-backend (backend)
>
> Let's make this a generic function so we can specialize it for certain
> backends.
>
> diff -r 5822fd897401 -r 826cbaa2b428 src/widgets/widget/widget.lisp
> --- a/src/widgets/widget/widget.lisp    Sat Jul 04 22:00:07 2009 +0300
> +++ b/src/widgets/widget/widget.lisp    Mon Jul 27 23:59:22 2009 +0300
> @@ -373,9 +373,9 @@
>  (defmethod render-widget (obj &rest args &key inlinep &allow-other-
> keys)
>    (declare (special *page-dependencies*))
>    (if (ajax-request-p)
> -    (mapc #'render-dependency-in-ajax-response (dependencies obj))
> -    (setf *page-dependencies*
> -         (append *page-dependencies* (dependencies obj))))
> +      (render-ajax-dependency-after-check (dependencies obj) *page-
> dependencies*)
> +      (setf *page-dependencies*
> +           (append *page-dependencies* (dependencies obj))))
>    (let ((*current-widget* obj))
>      (declare (special *current-widget*))
>      (if inlinep
> @@ -514,3 +514,10 @@
>    (timing (concatenate 'string "render-widget " (princ-to-string
> widget))
>      (call-next-method)))
>
> +(defun render-ajax-dependency-after-check (dependency-list page-
> dependency)
> +  "This helper function makes sure that only those dependencies that
> have not yet been set in the page-dependency are added through json."
> +  (loop for depend in dependency-list
> +     collect
> +       (if (member depend page-dependency)
> +          (render-dependency-in-ajax-response depend))))
> +
>
> Does this code have any effect? I think we don't have any page deps
> when
> we're in an AJAX request...
>
> diff -r 5822fd897401 -r 826cbaa2b428 test/weblocks-test.lisp
> --- a/test/weblocks-test.lisp   Sat Jul 04 22:00:07 2009 +0300
> +++ b/test/weblocks-test.lisp   Mon Jul 27 23:59:22 2009 +0300
> @@ -16,6 +16,12 @@
>
>  (in-package :weblocks-test)
>
> +;;Added the definition of the prototype backend so as to allow
> testing to continue given that weblocks no longer has a default setup.
> +(weblocks:define-client-backend prototype :js-root "backends/
> prototype"
> +                               :js-files `("prototype" "scriptaculous")
> +                               :css-root ""
> +                               :css-files `("layout" "main" "dialog"))
> +
>  (declaim (special *recovery-strategies*))
>
>  (defun call-with-test-environment (thunk)
>
> Hm, looks like this should be in core as well.
>
> diff -r 5822fd897401 -r 826cbaa2b428 pub/scripts/datagrid.js
> --- a/pub/scripts/datagrid.js   Sat Jul 04 22:00:07 2009 +0300
> +++ b/pub/scripts/datagrid.js   Mon Jul 27 23:59:22 2009 +0300
> @@ -1,3 +1,9 @@
> +////
> +
> +function throwAlertOnMissingBackend(){
> +  alert('Oops, we could not complete your request because of an
> internal error.');
> +
> +}
>
> The error message is wrong (should probably be "Function XXX not
> defined in
> this backend"), and this function has a lot of duplicates in other
> files, although
> it should be part of the common API.
>
> Lastly let's not use minified Javascript files. Later we can discern
> between
> production and debug mode and choose minified/vanilla versions based
> on that, but until then let's just use the plain files.
>
> That's it!
>
>   Leslie
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"weblocks" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/weblocks?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to