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

Consider exposing normalizeUIString #3174

Closed
bazineta opened this issue Sep 12, 2016 · 6 comments
Closed

Consider exposing normalizeUIString #3174

bazineta opened this issue Sep 12, 2016 · 6 comments

Comments

@bazineta
Copy link
Contributor

While the 3.x UIMixin exposes normalizeUIKeys and normalizeUIValues, normalizeUIString is private.

As an example below, a shim to provide @ui functionality to backbone.stickit, so that @ui values may be used in stickit automatic bindings, via the 'bindings' hash, and via manual binding, using addBinding().

(I've kept the shim in CS as I think it reads a bit more clearly; apologies to those who require therapy after viewing such things.)

As addBinding() can take a string as well as a hash, it's necessary to translate @ui strings as well as @ui hashes. As such, there's no alternative but to copy what Mn does internally in this case.

If normalizeUIString were exposed in the same manner that the other two normalize functions are, e.g.,

 normalizeUIString: function normalizeUIKeys(string) {
      var uiBindings = this._getUIBindings();
      return _normalizeUIString(string, uiBindings);
    }

Then this particular use case could be handled without duplicating private function.

#-----------------------------------------------------------------------------#
# Imports
#-----------------------------------------------------------------------------#

_          = require 'lodash'
Marionette = require 'backbone.marionette'

#-----------------------------------------------------------------------------#
# Save original Backbone.Stickit calls.
#-----------------------------------------------------------------------------#

stickit    = Marionette.View::stickit
addBinding = Marionette.View::addBinding
unstickit  = Marionette.View::unstickit

#-----------------------------------------------------------------------------#
# Return UI bindings for the provided view.
#-----------------------------------------------------------------------------#

ui = (view) -> _.result(view, '_uiBindings') or _.result(view, 'ui')

#-----------------------------------------------------------------------------#
# Attempt to perform @ui normalization of the provided string. As of this
# writing (Marionette 3.0.0), there's exposed function to normalize a hash,
# but not a bare selector, and we need to be able to normalize bare selectors
# to support addBinding() and unstickit().
#-----------------------------------------------------------------------------#

normalizeString = (string, view) ->
  return string.replace /@ui\.[a-zA-Z-_$0-9]*/g, (r) -> ui(view)[r.slice(4)]

#-----------------------------------------------------------------------------#
# Stickit selectors can be hashes, strings, or undefined. We need to
# normalize each type.
#-----------------------------------------------------------------------------#

normalizeSelector = (selector, view) ->
  return switch
    when _.isObject selector then view.normalizeUIKeys selector
    when _.isString selector then normalizeString selector, view
    else selector

#-----------------------------------------------------------------------------#
# Shim the three standard Stickit calls into Marionette View objects so
# that we can use the @ui syntax for bindings; this eliminates repetition of
# possibly long and complex selectors and allows us to have one definition
# for them, in the ui hash or function for the View.
#-----------------------------------------------------------------------------#

_.extend Marionette.View::,

  stickit: (optionalModel, optionalBindings) ->
    return stickit.call @,
      optionalModel,
      @normalizeUIKeys(optionalBindings or _.result(@, 'bindings') or {})

  addBinding: (optionalModel, selector, configuration) ->
    return addBinding.call @,
      optionalModel,
      normalizeSelector(selector, @),
      configuration

  unstickit: (optionalModel, optionalSelector) ->
    return unstickit.call @,
      optionalModel,
      normalizeSelector(optionalSelector, @)

#-----------------------------------------------------------------------------#
# Exports
#-----------------------------------------------------------------------------#

module.exports = Marionette

#-----------------------------------------------------------------------------#
@paulfalgout
Copy link
Member

👍 want to PR it? Needs some documentation, but that might be copy-pasta'd from v2.

@bazineta
Copy link
Contributor Author

Sure, I'll take a shot at it.

bazineta added a commit to bazineta/backbone.marionette that referenced this issue Sep 13, 2016
Expose normalizeUIString, to parallel normalizeUIKeys and
normalizeUIValues, issue marionettejs#3174.
bazineta added a commit to bazineta/backbone.marionette that referenced this issue Sep 13, 2016
Expose normalizeUIString, to parallel normalizeUIKeys and
normalizeUIValues. Issue marionettejs#3174.
@bazineta
Copy link
Contributor Author

#3179 is the code change required, which is trivial. (Ignore the closed #3178, which is me struggling to understand how to convince GitHub that I wanted next as the base, not master).

I can't seem to find documentation about the other two methods (normalizeUIKeys / normalizeUIValues), so I'm not sure where to place the documentation for this one.

@paulfalgout
Copy link
Member

Ah good point on the docs.. it looks like these were only ever documented when they were attached to Marionette itself, but when those were removed.. the docs went with it. So we can handle that separately. I assume though it'll still need a test.

@bazineta
Copy link
Contributor Author

Studying how to deal with a spec now.

@bazineta
Copy link
Contributor Author

Unit test added.

paulfalgout pushed a commit that referenced this issue Sep 14, 2016
* Expose normalizeUIString

Expose normalizeUIString, to parallel normalizeUIKeys and
normalizeUIValues. Issue #3174.

* Add normalizeUIString unit test

Update test coverage to handle normalizeUIString.
paulfalgout pushed a commit to paulfalgout/backbone.marionette that referenced this issue Sep 26, 2016
* Expose normalizeUIString

Expose normalizeUIString, to parallel normalizeUIKeys and
normalizeUIValues. Issue marionettejs#3174.

* Add normalizeUIString unit test

Update test coverage to handle normalizeUIString.
paulfalgout pushed a commit to paulfalgout/backbone.marionette that referenced this issue Oct 12, 2016
* Expose normalizeUIString

Expose normalizeUIString, to parallel normalizeUIKeys and
normalizeUIValues. Issue marionettejs#3174.

* Add normalizeUIString unit test

Update test coverage to handle normalizeUIString.
ahumphreys87 pushed a commit that referenced this issue Oct 13, 2016
* Expose normalizeUIString

Expose normalizeUIString, to parallel normalizeUIKeys and
normalizeUIValues. Issue #3174.

* Add normalizeUIString unit test

Update test coverage to handle normalizeUIString.
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