Open
Conversation
added 30 commits
November 21, 2014 12:08
Make the implementation file visible to Jasmine
I don't want to use jQuery. A Chrome extension dependent on it can't stand alone.
Don't have anonymous Objects, prefer classes.
It's unusual to sniff based on the `$.ajax` `statusCode` property. It's clearer to have the generalized `always()` callback interpret the returned `jqXHR` arguments. We use a dispatch table (`statusCodeMap`) to find a function and execute it.
* Remove repeated template code * Simplify pseudo-templating using join() * Remove statusCodeMap * Remove default `GET` verb specification
Decouple URL testing from DOM updating
Move onto Bookmark, we were passing bookmark instance multiple times, let the instance know how to do this.
Rendering template is not really part of `urlTesting`.
Focus method on determining the HTTP status code.
Bookmark now determines its own status. If an HTML representation is asked for, we return a `$.Deferred` which is later resolved with the full template representation.
`treeWalk()` had a check for an unset global. Prefer to explicitly initialize the global, `bookmarksArray` and explicitly name it as being on `window`.
This function is concerned with a recursive search and display of the `Bookmark`s located within `Object` containers. This sets the stage for `Object` containers to become some sort of a class.
Chrome's bookmarks collection is composed of leaf nodes (Bookmarks) and containers. Introduce a class that owns the recursive searching and aggregation of contained sub-nodes.
added 30 commits
January 10, 2015 10:29
Add a method that simplifies display of the View data.
`getBookmarks()` was a bit lazy.
_initializeDeletionControls
I'm uncomfortable with the dependencies that are in this app that use jQuery: * Dialog box * Selection behavior * Ajax() behavior All of these are core to the app and can be performed from within the context of Chrome's very-capable JS engine. They map to: * confirm() * document.querySelector* * Promise() I'd like to continue this work to remove the jQuery links. It feels like an unnecessary tangle to keep jQuery in here.
This reverts commit b069e60. Let's start pulling jQuery out.
Use built-in confirm() dialog. Remove sizable (byte-wise) dependency on a library that creates a network dependency.
Replacement for `$.ajax()`.
I've tried, unsuccessfully, to figure out how to test `Promise`s. Most of the ways of testing them seem to involve some or several of the following * _more_ promises invoking the callback * manual invocation of callback state changes
This snarl of POJsOs was pretty hairy. Try to clean things up a little bit.
Doesn't seem to be needed to work.
Doesn't seem to be used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey Erica,
If you'll recall I was talking about this code with you a few month back (right before my move to NYC).
I've not had much time to work on it these last few weeks (er, months) but I finally had some time to put it all together. I wanted to try using Jasmine to write some tests around this code and to try to do some step-wise refactoring. Let me know what you think / if you want to merge it.
Steven