Imager 0.1.0 #49

thom4parisot · 2013-12-06T14:11:11Z

At last!

Conflicts: test/unit/core.js

…ternal dollar and tests Conflicts: test/unit/core.js

- initial tests are failing because the old feature is still working (good way to know the old expectation we are going to break) - new tests are not working because basically it's not done yet

- removed Imager `regex` option - implemented simple `{width}` interpolation

- added Imager.transforms to let room to customise things further on - replacing {pixel_ratio} pattern in `data-src` URI

Also made the pixel ratio computation executed once, not every time an image is replaced (because it is not supposed to change within the lifecycle of a page).

- list of authors - licensing - created bower package - made `imager` available for publication in npm (it is relevant to use it to host web component on npm) - created the AUTHOR file for npm authors list

…ternal dollar and tests

And making sure we detach any HTML/Array-like collection.

- moved event setup to init - created dedicated method to register events whenever you want

- JS methods documentation - late warning about a possible `scrollDelay` value set to `0`

thom4parisot · 2013-12-06T15:07:19Z

Hu hu the fact it is again a 55 commits PR is a nice coincidence 8-)

MoOx · 2013-12-06T18:09:26Z

Integralist · 2013-12-07T17:57:37Z

Imager.js

+        pixelRatio: function(value){
+            return value === 1 ? '' : '-'+value+'x';
+        },
+        width: function transformWidth(width, map){


@oncletom just wondering if this function needs to be named? It doesn't look like it's used by anything so it could be anonymous right?

Integralist · 2013-12-07T18:03:17Z

@oncletom heya, just ran npm install && npm run test and got the following output, wasn't sure if this was expected or not (in that, although there are 404 errors, ultimately it looks like each Phantom check is succeeding)?

❯ npm run test

> imager@0.1.0 test /Users/Code/Imager
> jshint Imager.js && karma start --single-run

INFO [karma]: Karma v0.10.8 server started at http://localhost:9876/
INFO [launcher]: Starting browser PhantomJS
INFO [PhantomJS 1.9.2 (Mac OS X)]: Connected on socket tY9nEXRUDwGAklr2ONt3
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/A-320.jpg
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/C-320.jpg
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/B-320.jpg
PhantomJS 1.9.2 (Mac OS X): Executed 2 of 17 SUCCESS (0 secs / 0.029 secs)
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/C-320.jpg
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/B-320.jpg
PhantomJS 1.9.2 (Mac OS X): Executed 3 of 17 SUCCESS (0 secs / 0.05 secs)
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/C-320.jpg
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/B-320.jpg
PhantomJS 1.9.2 (Mac OS X): Executed 4 of 17 SUCCESS (0 secs / 0.071 secs)
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/C-320.jpg
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/B-320.jpg
PhantomJS 1.9.2 (Mac OS X): Executed 5 of 17 SUCCESS (0 secs / 0.092 secs)
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/C-320.jpg
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/B-320.jpg
PhantomJS 1.9.2 (Mac OS X): Executed 9 of 17 SUCCESS (0 secs / 0.237 secs)
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/B-640.jpg
PhantomJS 1.9.2 (Mac OS X): Executed 10 of 17 SUCCESS (0 secs / 0.259 secs)
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/A-320.jpg
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/C-320.jpg
PhantomJS 1.9.2 (Mac OS X): Executed 15 of 17 SUCCESS (0 secs / 0.284 secs)
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/A-320.jpg
WARN [web-server]: 404: /base/Demo%20-%20Grunt/Assets/Images/Generated/C-320.jpg
PhantomJS 1.9.2 (Mac OS X): Executed 17 of 17 SUCCESS (0.715 secs / 0.485 secs)

Integralist · 2013-12-07T18:06:16Z

Imager.js

@@ -39,10 +60,6 @@
                // Class name to give your resizable images.
                className: '',

-                // Regular expression to match against your image endpoint's naming conventions
-                // e.g. http://yourserver.com/image/horse/400
-                regex: RegExp


@oncletom I think the only config option missing is the onResize property. Would you be OK to add that please.

Are you referring to the onResize from #47?

@oncletom I'm referring to the config option onResize which can optionally be specified when creating a new Imager instance (see: https://github.com/BBC-News/Imager.js/blob/b7b31d795ff9d7a0cb39a9626ca426299007a1d2/README.md#onresize) - just thought that should probably be specified in that code comment block as the other config options are detailed there.

I'm going to add this myself now, but I'll just use what you've already written on the README :-)

Oh yes, that's true, forgot to add it in the JS comment!

@oncletom I've added this now, see: https://github.com/BBC-News/Imager.js/pull/49/files#diff-b33c92ac11fbf0d3ea67899762af99b7R63

Integralist · 2013-12-07T20:11:40Z

README.md

+```js
+var imgr = new Imager();
+
+$(document).on('load.lowPriority', $.proxy(imgr.registerResizeEvent, imgr));


@oncletom can you clarify the load.lowPriority?

It is a fictional event on which someone could hook into to register the resizing event. This was the only idea I had at the time of writing the doc.

However if you think there is an existing DOM event which would be more suitable and relevant to register onto, I'm buying the idea!

Integralist · 2013-12-07T20:33:52Z

README.md

+var imgr = new Imager();
+
+// Using jQuery to set-up the event handling and help keep the correct scope when executing the callback
+$(document).on('load.lowPriority', $.proxy(imgr.registerResizeEvent, imgr));


@oncletom can you clarify the load.lowPriority?

Integralist · 2013-12-07T20:36:13Z

@oncletom I've made some tweaks (see last three commits). There are a couple of outstanding comments I've made (see above). If you could comment back please and then once that's done I'll give a final review and then merge :-)

Integralist · 2013-12-08T13:18:45Z

@oncletom fixed that typo, whoops! Good spot :-)

thom4parisot · 2013-12-08T13:23:11Z

No problem! Also the 404 in the tests are related to the fact they have not been generated by the grunt responsive images task. This is not making the tests failing, just raising a notice.

We could either run grunt responsive_images before the tests are launched or ship dedicated test images, independant from the demo.

Integralist · 2013-12-08T13:30:16Z

@oncletom I reckon we ship dedicated test images, as it'll mean running the tests will be faster not having to rely on the grunt responsive_images task to run. Is this something you want to do now or wait until this get's merged? It's not a deal breaker by any means.

Integralist · 2013-12-08T13:46:07Z

@oncletom regarding load.lowPriority I wonder whether it's worth changing that to just load (e.g. $(document).on('load', callback);) so it doesn't confuse people into thinking load.lowPriority is a real event.

thom4parisot · 2013-12-08T13:46:52Z

We'll do that later then. Maybe in the same PR as moving all the demos in a dedicated folder.

Regarding the load.lowPriority, agreed, let's keep things simple by avoiding adding possible confusion.

Integralist · 2013-12-08T13:47:54Z

@oncletom coolio. I've just updated the event btw

Integralist · 2013-12-08T13:49:29Z

@oncletom I think this is good to merge now. You happy for me to push the big green button? :-)

thom4parisot · 2013-12-08T13:55:51Z

@Integralist oh I appreciate but please do it, I like the symbol and the positive progress as an outcome from my first attempt :-) 👍

Imager 0.1.0

thom4parisot added 30 commits December 6, 2013 14:12

Test framework and jshint adjustments

75e06c0

We don't even use grunt for testing

4944ca7

Testing constructor.

5ca9e89

Testing new Constructor API (tests are obviously failing)

b6eeab6

Handling images in fixtures

9b06270

Conflicts: test/unit/core.js

Removing $ from the global namespace to avoid conflicting with the in…

841f226

…ternal dollar and tests Conflicts: test/unit/core.js

Testing new data-src syntax.

ee71446

- initial tests are failing because the old feature is still working (good way to know the old expectation we are going to break) - new tests are not working because basically it's not done yet

regex -> {width}

65ec535

- removed Imager `regex` option - implemented simple `{width}` interpolation

Pixel ratio tests

b125a4b

Pixel ratio implementation

28720dc

- added Imager.transforms to let room to customise things further on - replacing {pixel_ratio} pattern in `data-src` URI

Fixed typo in test label.

30abfec

Made timeout execution more explicit

aaefc1a

pixel_ratio -> pixelRatio as method name.

cf66a9f

Also made the pixel ratio computation executed once, not every time an image is replaced (because it is not supposed to change within the lifecycle of a page).

Setup to publish as imager on npm and bower registries.

8a73896

- list of authors - licensing - created bower package - made `imager` available for publication in npm (it is relevant to use it to host web component on npm) - created the AUTHOR file for npm authors list

Codestyle consistency

07a4a68

Testing width interpolation

d8e805d

Force sorting to avoid unordered sizes.

2a13974

Using a Hashmap instead of an array of objects.

a11fc3a

Safer way to check array type

b772418

Testing internals and combinations of elements with/without options

1f65e01

Removing $ from the global namespace to avoid conflicting with the in…

72817c5

…ternal dollar and tests

Handling images in fixtures

2bab1d2

First draft of implementation, not working yet.

64ced5e

Fixed HTML issue breaking tests

7ce6347

Cleaning fixtures is a good idea.

9b0ba22

And making sure we detach any HTML/Array-like collection.

(Array.prototype|[]).slice.call -> applyEach w/ callback function

f58a193

Fixed merge issues (fixtures and duplicate afterEach block)

6599b97

returnFn -> returnDirectValue

1cc0bf8

Event tests

3118030

Splitting events registration to hook tests on them

f323667

- moved event setup to init - created dedicated method to register events whenever you want

thom4parisot added 4 commits December 6, 2013 14:17

JavaScript options

6ef2365

Examples for HTML API

b5eaf2b

new Imager doc

68100e7

JS API documentation.

6e89f97

- JS methods documentation - late warning about a possible `scrollDelay` value set to `0`

thom4parisot mentioned this pull request Dec 6, 2013

[READY] Refactoring Plan #32

Merged

12 tasks

Added contact details for all authors

15d06b0

Integralist reviewed Dec 7, 2013
View reviewed changes

Minor formatting tweaks

02757c7

Integralist reviewed Dec 7, 2013
View reviewed changes

Tweaks to the README

1bb263a

Integralist reviewed Dec 7, 2013
View reviewed changes

Typo

b7b31d7

Update code comment example to include onResize option

181e8c4

Change event

76d33f5

Integralist added a commit that referenced this pull request Dec 8, 2013

Merge pull request #49 from BBC-News/refactoring-base

9fc85eb

Imager 0.1.0

Integralist merged commit 9fc85eb into master Dec 8, 2013

thom4parisot deleted the refactoring-base branch December 8, 2013 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Imager 0.1.0 #49

Imager 0.1.0 #49

thom4parisot commented Dec 6, 2013

thom4parisot commented Dec 6, 2013

MoOx commented Dec 6, 2013

Integralist Dec 7, 2013

Integralist commented Dec 7, 2013

Integralist Dec 7, 2013

thom4parisot Dec 8, 2013

Integralist Dec 8, 2013

thom4parisot Dec 8, 2013

Integralist Dec 8, 2013

thom4parisot Dec 8, 2013

Integralist Dec 7, 2013

thom4parisot Dec 8, 2013

Integralist Dec 7, 2013

Integralist commented Dec 7, 2013

Integralist commented Dec 8, 2013

thom4parisot commented Dec 8, 2013

Integralist commented Dec 8, 2013

Integralist commented Dec 8, 2013

thom4parisot commented Dec 8, 2013

Integralist commented Dec 8, 2013

Integralist commented Dec 8, 2013

thom4parisot commented Dec 8, 2013

Imager 0.1.0 #49

Imager 0.1.0 #49

Conversation

thom4parisot commented Dec 6, 2013

thom4parisot commented Dec 6, 2013

MoOx commented Dec 6, 2013

Choose a reason for hiding this comment

Integralist commented Dec 7, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Integralist commented Dec 7, 2013

Integralist commented Dec 8, 2013

thom4parisot commented Dec 8, 2013

Integralist commented Dec 8, 2013

Integralist commented Dec 8, 2013

thom4parisot commented Dec 8, 2013

Integralist commented Dec 8, 2013

Integralist commented Dec 8, 2013

thom4parisot commented Dec 8, 2013