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

Bounded Binding: Remove old bind strategy #1568

Merged
merged 32 commits into from Jun 18, 2014

Conversation

Projects
None yet
3 participants
@Shadowfiend
Copy link
Member

Shadowfiend commented May 27, 2014

#1548 reopened with respect to lift_30. This is now ready for review.

The PR blows away bind and most things that came with it. It preserves a few functions
that were in BindHelpers that were related to more general HTML manipulation, which
now live in an HtmlHelpers trait instead. It also rearranges the ToCssBindPromoter
implicits so that they are implicit classes and in a single centralized place (a
CssBindImplicits trait). These are mixed into HtmlHelpers and through there into
Helpers, so importing Helpers._ will continue to import the appropriate implicit
conversions.

The second thing this does is rewrite few points where we were still leaning on
the old binding strategy, particularly PaginatorSnippet, the Menu snippet, mapper
view support stuff, LiftScreen, and lift-proto's Crudify and ProtoUser. The
snippets themselves may not be optimal now; however, I made the rewriting function
very similarly to how it functioned before (using CSS classes instead of the element
names that the binding style was using), leaving decisions on whether the snippets
should be further reworked for another time.

Done/pending work:

  • Remove BindHelpers and BindHelpersSpec
  • Move still-used generic stuff to HtmlHelpers
  • Preserve BindHelpersSpecs for HtmlHelpers stuff in HtmlHelpersSpec
  • Port snippets still using bind to CSS selector transforms
  • Put implicits for ToCssBindPromoter in a common place
  • Ensure untested stuff in HtmlHelpers is tested
  • Make LiftScreen compile without bind
  • Promote CssBoundLiftScreen to LiftScreen
  • Make lift-proto compile without bind
  • Make lift-mapper compile without bind
  • Make sure all specs are passing

Shadowfiend and others added some commits Apr 19, 2014

Remove BindHelpers and BindPlus for HtmlHelpers.
HtmlHelpers keeps just the bare minimum of non-old-bind-
related functionality from BindHelpers, things related directly
to manipulating NodeSeqs rather than to bindings.

Note that while util compiles, webkit emphatically does not.
Work is forthcoming to fix that.
Bring back AttrHelper.
This is used fairly extensively in S, and isn’t directly tied to
the old binding approach.
Turn ToCssBindPromoter to implicit classes.
Before, we had a couple of implicit conversions from String to
ToCssBindPromoter, which was a final case class. We now
have a CssBindImplicits trait that defines a CssBindPromoter
class and two implicit classes that extend it, one for strings
and one for CssSelector instances.

These implicits are mixed into HtmlHelpers so they are
available, as before, when Helpers._ is imported.
Fix a higherKinds warning.
Also add a FIXME to verify whether the higher kinded
AttrHelper type parameter is needed.
Move lift:embed to CSS selector transforms.
It now operates on ids, not lift:bind-at elements.
Fix a lost implicit Str->JsExp conversion.
Presumably this was indirecting through a BindHelpers implicit
and ending up as a JsExp somehow…
Change Menu.group to return a CssSel.
Since we’re yielding a top-level CssSel, this lets folks decide
whether to combine using & or andThen when referencing the
snippet directly.
Reimplement PaginatorSnippet.paginate using CssSel.
Also SortedPaginatorSnippet’s override of .paginate.
Restore find(Box|Option|Id) to HtmlHelpers.
These are generic HTML helpers rather than relying on
anything binding-related, and they’re used in other places in
the codebase.
Restore deepEnsureUniqueId to HtmlHelpers.
Also unify most of its implementation with that of
ensureUniqueId.
Preserve applicable BindHelpersSpecs.
These are now in an HtmlHelpersSpec singleton.
BindHelpersSpec goes away completely.
Fix CssSelectorSpec for lost BindHelpers.
We now import all of Helpers, which provides the appropriate
implicits and such.
Remove pagesXml(pages, sep).
It’s the same as pagesXml(pages)(sep), and having both
doesn’t seem particularly necessary.
Make ProtoUser compile without bind.
Not the cleanest move to CSS selector transforms, but one
that should work.
Make Crudify compile without bind.
Lot of CSS selector magic throughout, also helped clean up
a lot of NodeSeq=>NodeSeq functions to just return an
appropriate CSS selector transform.
Fix an issue with mapper's use of new paginator.
Before paginator.paginate needed a _ to get a
NodeSeq=>NodeSeq; that is no longer necessary.
Remove FormProcessor.
There doesn’t seem to be any documentation describing
how to truly use it, and it was introduced as an
“experimental” feature. If folks ask for it back, we can
restore it after inquiring as to how they’re using it.

Shadowfiend added some commits May 27, 2014

Fix NamedCometPerTabSpec.
This was broken because comet actors used to extend
BindHelpers, which pulled in CSS selector implicits. We
have CometActor mix in CssBindImplicits now so that these
are in scope inside comets.

@farmdawgnation farmdawgnation added this to the 3.0-M2 milestone Jun 7, 2014

@farmdawgnation

This comment has been minimized.

Copy link
Member

farmdawgnation commented Jun 7, 2014

Nothing in the code jumps out at me, but this also uses large swatches of the framework that I don't interact with frequently. Going to verify the test locally, but would love another set of eyes that are more familiar with mapper, screen, and proto in particular.

@farmdawgnation

This comment has been minimized.

Copy link
Member

farmdawgnation commented Jun 7, 2014

From the state of Denmark a suspicious scent does waft.

[error] Failed: : Total 41, Failed 2, Errors 0, Passed 39, Skipped 0
[error] Failed tests:
[error]     net.liftweb.mongodb.MongoDocumentExamplesSpec
[error]     net.liftweb.mongodb.MongoDirectSpec
[error] (lift-mongodb/test:test) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 31 s, completed Jun 7, 2014 11:38:19 AM
@Shadowfiend

This comment has been minimized.

Copy link
Member Author

Shadowfiend commented Jun 7, 2014

Lulwut. There are no changes in there haha. I'll look into it, may be a merge issue.

@Shadowfiend

This comment has been minimized.

Copy link
Member Author

Shadowfiend commented Jun 7, 2014

Reporting back, these specs are running successfully for me repeatedly, so… Dunno what's up here? Maybe a clean is needed?

@farmdawgnation

This comment has been minimized.

Copy link
Member

farmdawgnation commented Jun 7, 2014

Hmmmmm. Could be something is up with my local mongo. I'll see what I can figure out later this afternoon.

@farmdawgnation

This comment has been minimized.

Copy link
Member

farmdawgnation commented Jun 7, 2014

Yep, this is because Mongo changed some error output on their end. So, this is good by me. I'll open a bug separately to deal with that. 👍

@Shadowfiend

This comment has been minimized.

Copy link
Member Author

Shadowfiend commented Jun 18, 2014

Hokies, seeing one pass, going to merge this guy tomorrow if no one brings up any reservations.

@Shadowfiend

This comment has been minimized.

Copy link
Member Author

Shadowfiend commented Jun 18, 2014

Doin' it.

Shadowfiend added a commit that referenced this pull request Jun 18, 2014

Merge pull request #1568 from lift/bounded-binding
Bounded Binding: Remove old bind strategy

The PR blows away bind and most things that came with it. It preserves a few functions
that were in BindHelpers that were related to more general HTML manipulation, which
now live in an HtmlHelpers trait instead. It also rearranges the ToCssBindPromoter
implicits so that they are implicit classes and in a single centralized place (a
CssBindImplicits trait). These are mixed into HtmlHelpers and through there into
Helpers, so importing Helpers._ will continue to import the appropriate implicit
conversions.

The second thing this does is rewrite few points where we were still leaning on
the old binding strategy, particularly PaginatorSnippet, the Menu snippet, mapper
view support stuff, LiftScreen, and lift-proto's Crudify and ProtoUser. The
snippets themselves may not be optimal now; however, I made the rewriting function
very similarly to how it functioned before (using CSS classes instead of the element
names that the binding style was using), leaving decisions on whether the snippets
should be further reworked for another time.

@Shadowfiend Shadowfiend merged commit aa429b2 into lift_30 Jun 18, 2014

@Shadowfiend Shadowfiend deleted the bounded-binding branch Jun 18, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment