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

Options UI #39

Merged
merged 40 commits into from Jun 29, 2018
Merged

Options UI #39

merged 40 commits into from Jun 29, 2018

Conversation

developit
Copy link
Collaborator

@developit developit commented May 25, 2018

Implements basic support for options (#30), configurable for both the left and right panels. One of the modes is "original", which is a pass-through that displays the source image.

export type CodecOptions = any;

type Encoders = {
[type: string]: any // @todo Encoder
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a "TypeScript is frustrating" moment. For some reason, MozJpegEncoder isn't considered to have the same interface as Encoder? It's likely just me doing something stupid.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MozJpegEncoder doesn’t implement Encoder, an instance of MozJpegEncoder does 😄

You could do new () => Encoder instead of any here.

Copy link
Collaborator Author

@developit developit May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see, I knew it was just me being dumb. fixed.

@developit
Copy link
Collaborator Author

Aside from the design obviously being poor, I'd like to collocate the options component for each codec with the codec itself. I'm thinking that will be best if I move them into src/codecs/[codec]/{index,options}.ts

Copy link
Collaborator

@surma surma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of things.

One bigger question is now we handle scrolling. I think we should try and stick to the root scroller, as iOS will always feel uncanny otherwise.

width: 100%;
height: 100%;
overflow: hidden;
contain: strict;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

top: 0;
width: 100%;
height: 100%;
overflow: hidden;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t feel like we should go overflow: hidden without a good reason. For mobile we definitely want to be using the root scroller and this would make that very hard.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to write down what we talked about in the office: I think we should use per-element scrollers for our compression option elements. If iOS gives us troubles, we'll work around them (potentially by reducing niceness on that platform). Basically, I'm sick of scrolling being broken on iOS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also trying to use fixed for the fixed portions of the UI would be a nightmare with auto-hiding address bar.

.app h1 {
color: green;
.app {
position: absolute;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this file could be condensed down a lot with flexbox and grid.

rightOptions: {}
};

optionsUpdateTimer?: NodeJS.Timer | number | null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Don’t think you need null if you are using ?
  • Why NodeJS.Timer?
  • What does this prop stand for?

Copy link
Collaborator

@jakearchibald jakearchibald Jun 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Same questions here.

Copy link
Collaborator Author

@developit developit Jun 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only works with this combination. I believe it's because we're combining the Node and web global typescript defs - setTimeout() returns a NodeJS.Timer, but cancelTimeout expects an int.

  • update: removed entirely


compressCounter = 0;

retries = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think we should be doing retries. Either it’s a bug in the way I wrote or invoke my C code, or it’s a Chrome bug. If it’s a Chrome bug and it can’t be fixed, only then should we consider writing a retry strategy, and it should be modular instead of being part of the app container.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

Copy link
Collaborator Author

@developit developit Jun 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Removed.

box-shadow: inset 0 0 1px #fff, 0 0 1px #fff;
border-radius: 3px;
color: #eee;
overflow: auto;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should try to avoid scrolling elements as much as possible.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm this as per @jakearchibald’s comment

}
updateCanvas(canvas: HTMLCanvasElement, img?: ImageBitmap) {
let ctx = canvas.getContext('2d');
if (!ctx) return;
Copy link
Collaborator

@surma surma May 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Silent errors are evil, please handle that the same way as the old code.

width: 100%;
height: 100%;
overflow: hidden;
contain: strict;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other file

@@ -1,5 +1,5 @@
export interface Encoder {
encode(data: ImageData): Promise<ArrayBuffer>;
encode(data: ImageData, options: any): Promise<ArrayBuffer>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jake and I discussed in the original PR as well and I said I’d like to keep the encode() function consistent across all codecs and have individual methods for each option.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy for it to land as-is, and we can have a discussion when we start to integrate options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure what we wanted the lifecycle of these encoder classes to be. If we reuse one instance for multiple encodes, methods are pain. Also given the fact that state is managed elsewhere, duplicating it here just causes extra work.

const m = await this.emscriptenModule;
const api = await this.api;

const p = api.create_buffer(data.width, data.height);
m.HEAP8.set(data.data, p);
api.encode(p, data.width, data.height, 2);
api.encode(p, data.width, data.height, options.quality!=null ? options.quality : 7);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should throw (or something) if a required option is not given. The UI can define a default, the module itself shouldn’t imo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with that, or have it default to 100

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing if quality is missing seems like the right rule for now. We can revisit this as we add more compression options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

jpeg: (options: CodecOptions) => `JPEG ${options.quality || ''}`
};

const AllEncoders: Encoders = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type-checks the values in the object, but it also means AllEncoders.jpeg is of type Encoder rather than MozJpegEncoder. Is that intentional?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the motivation for not having an options parameter for the encode() call, so that all encoders could be handled transparently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the types entirely for now.

[type: string]: new () => Encoder
};

const EncoderNames = {
Copy link
Collaborator

@jakearchibald jakearchibald Jun 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer encoderNames for this (lower case e), since it isn't a class/interface/type. I could be persuaded to use ENCODER_NAMES since it's a top-level constant.

@kosamari @surma any opinions here?

  • encoderNames

};

const EncoderNames = {
original: 'Original Image',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My feeling is that these could all become just plain strings. Changing the same based on the settings works now, but won't make sense once we add the other settings.

leftType: ImageType,
rightType: ImageType,
leftOptions: CodecOptions,
rightOptions: CodecOptions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General question on state objects: There's a lot of left* right* here, so my instinct is to group them in "left" and "right" properties. Is that kind of nesting bad practice in preact?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessarily bad practice, but it can make it more difficult to implement shouldComponentUpdate intelligently, and people tend to avoid storing objects in state because mutating them in-place can break shouldComponentUpdate entirely (old===new). I'm not sure we care that much, so I'm open to nesting to make things cleaner. It would make the methods to update an option simpler, and would also help avoid coupling the whole app to a left/right UI (maybe we have 4 panels down the line).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. In terms of shouldComponentUpdate, we could agree to changing the parent if a property needs changing, immutable-style. I think there are libraries to help with that. Do you have a favourite?

Copy link
Collaborator Author

@developit developit Jun 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

immutability-helper is decent. There's also immer, but I've not yet used it in anything.

rightOptions: {}
};

optionsUpdateTimer?: NodeJS.Timer | number | null;
Copy link
Collaborator

@jakearchibald jakearchibald Jun 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Same questions here.

onInput={updateOption}
/>
</label>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indenting above

@@ -0,0 +1,34 @@
.options {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As before, slap a "this is temporary" comment at the top and I'm happy for this to merge.

ctx.clearRect(0, 0, canvas.width, canvas.height);
ctx.drawImage(img, 0, 0);
}
updateCanvas(canvas: HTMLCanvasElement, img?: ImageBitmap) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look like this needs to be a member of the class (no state usage)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it’s a good utility function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • move to utility function

@@ -1,5 +1,5 @@
export interface Encoder {
encode(data: ImageData): Promise<ArrayBuffer>;
encode(data: ImageData, options: any): Promise<ArrayBuffer>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy for it to land as-is, and we can have a discussion when we start to integrate options.

@@ -5,6 +5,8 @@ html, body {
width: 100%;
padding: 0;
margin: 0;
font: 14px/1.3 Roboto,'Helvetica Neue',arial,helvetica,sans-serif;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move these changes into their own rule and put the usual disclaimer there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could just assume all of the styling is temporary, right? we don't have a completed design.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair for this resource, but some styles (such as image-rendering) are intentional.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

Copy link
Collaborator

@jakearchibald jakearchibald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't mean to approve in my previous review

@developit
Copy link
Collaborator Author

Alright, let me know if anything else is needed!

Copy link
Collaborator

@jakearchibald jakearchibald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address the issues and I'm happy for this to merge.

Consider this branch feature-frozen though. We should tackle additional stuff in PRs.

@@ -1,5 +1,5 @@
export interface Encoder {
encode(data: ImageData): Promise<ArrayBuffer>;
encode(data: ImageData, options: any): Promise<ArrayBuffer | ImageData>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid any unless we really can't. IMO this should be:

export interface Encoder<OptionsType> {
  encode(data: ImageData, options: OptionsType): Promise<ArrayBuffer | ImageData>;
}

Also ArrayBuffer | ImageData seems weird. Is this for the "original" encoder. It's probably better to treat "original" as something other than an encoder than mess up the interface.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can treat original as an encoder. ImageData doesn’t really make sense, so they should all use ArrayBuffer imo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@surma if you can get that to work, I agree. I couldn't.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • generics for encoder options


type EncodeOptions = {};

export default class IdentityEncoder implements Encoder {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would become

export default class IdentityEncoder implements Encoder<EncodeOptions> {


type Props = {
options: CodecOptions,
updateOption(e: Event): void,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For event-driven callbacks, I tend to name them onWhatever. Is this a good convention? +@surma.


export type ImageType = 'original' | 'MozJpeg';

export type CodecOptions = any;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid an any type here, but I'm happy for this to merge as long as an issue is created to fix this (I'm happy to do the fixing).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done the simplest thing for now:

export type CodecOptions = IdentityEncodeOptions | MozJpegEncodeOptions;

@@ -268,7 +268,7 @@ export default class PinchZoom extends HTMLElement {
}

/** Transform the view & fire a change event */
private _applyChange (opts: ApplyChangeOpts = {}) {
private _applyChange(opts: ApplyChangeOpts = {}) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was using the standardjs style, but I dislike the space after the function name,

What lint rules are you using? Check them in.

Copy link
Collaborator Author

@developit developit Jun 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rules are in our package.json, so my editor is enforcing them any time I touch a file. Surma's PR swaps them out for tslint.

@@ -1,5 +1,5 @@
export interface Encoder {
encode(data: ImageData): Promise<ArrayBuffer>;
encode(data: ImageData, options: any): Promise<ArrayBuffer | ImageData>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can treat original as an encoder. ImageData doesn’t really make sense, so they should all use ArrayBuffer imo.

package.json Outdated
@@ -83,6 +83,9 @@
},
"dependencies": {
"classnames": "^2.2.5",
"comlink": "^3.0.3",
"comlink-loader": "^1.0.0",
"immunize": "git+https://gist.github.com/9ee47f9a6122669b9a1b77587af04ff8.git",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed :P


export type CodecOptions = any;

const ENCODER_NAMES = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment where this string is used. Also, can we merge ENCODER_NAMES and ENCODERS or do they need to be separate?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And/or declare a type (especially if the two merge)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • merge ENCODERS & ENCODER_NAMES

MozJpeg: MozJpegEncoder
};

type Image = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I prefer interface over type, as they can be extended.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for some other files in this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • interfaces

sourceImg?: ImageBitmap,
sourceData?: ImageData,
images: Array<Image>,
loading: boolean,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is the app state, can we add some comments explaining the props? Like, what does loading stand for and what is the desired effect?

}

render({ }: Props, { loading, error, images }: State) {
for (let image of images) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loading = images.any(image => image.loading)? :3


@bind
setOption(key: string, value?: any) {
const options = Object.assign({}, this.state.options);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering: should we be using immunize for all these state-copy-thingies for consistency?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe. I actually just removed it, but if it's cleaner and everyone is on board I'm find adding it back in.

ctx.clearRect(0, 0, canvas.width, canvas.height);
ctx.drawImage(img, 0, 0);
}
updateCanvas(canvas: HTMLCanvasElement, img?: ImageBitmap) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it’s a good utility function.

@@ -97,6 +98,10 @@ module.exports = function (_, env) {
}
]
},
{
test: /\.worker.[tj]sx?$/,
loader: 'comlink-loader'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@@ -111,16 +116,16 @@ module.exports = function (_, env) {
{
// All the codec files define a global with the same name as their file name. `exports-loader` attaches those to `module.exports`.
test: /\/codecs\/.*\.js$/,
loader: 'exports-loader',
loader: 'exports-loader'
},
{
test: /\/codecs\/.*\.wasm$/,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this rule can be removed now, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasn't sure - are we not using it anymore?

ctx.drawImage(img, 0, 0);
}
updateCanvas(canvas: HTMLCanvasElement, img?: ImageBitmap) {
let ctx = canvas.getContext('2d');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@jakearchibald
Copy link
Collaborator

@developit @surma @kosamari Ready for review! I've also rebased on master.

[mozJPEG.type]: mozJPEG
};

export const encoders = Array.from(Object.values(encoderMap));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to edit this file as we add new encoders. It isn't as DRY as I'd like, but I couldn't get TypeScript to do better.

// Encoder has been comlinked.
const encoder = new Encoder() as any as Promise<Encoder>;
return (await encoder).encode(data, options);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All encoders will export these things, except identity which doesn't encode, or have a static extensions / mime type etc.

// Encoder has been comlinked.
const encoder = new EncoderWorker() as any as Promise<EncoderWorker>;
return (await encoder).encode(data, options);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All encoders will export these things, except identity which doesn't encode, or have a static extensions / mime type etc.

</div>
);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed state from this component. The form elements are still controlled, but by the app.

export async function encode(data: ImageData, options: EncodeOptions = {}) {
// This is horrible because TypeScript doesn't realise that
// Encoder has been comlinked.
const encoder = new Encoder() as any as Promise<Encoder>;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah that sucks. I had wondered about having encoders export an async factory function instead just to make the worker boundary transparent.

counter: image.counter + 1
encoderState,
loading: true,
counter: image.counter++
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++image.counter ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Will fix.

Copy link
Collaborator

@surma surma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor things, otherwise lgtm

export async function encode(data: ImageData, options: EncodeOptions) {
// This is horrible because TypeScript doesn't realise that
// Encoder has been comlinked.
const encoder = new EncoderWorker() as any as Promise<EncoderWorker>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn’t await new EncoderWorker() just work?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh good point.

const result = await this.updateCompressedImage(source, image.encoderState);
image = this.state.images[index];
// If another encode has been initiated since we started, ignore this one.
if (image.counter !== id) return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I’m misunderstanding, but if image.counter < id it means an older encoding job has finished and we shouldn’t early return?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I messed up the counter pretty well. Will fix.

@jakearchibald
Copy link
Collaborator

I've created two counters, one for loading and one for loaded. This means we can render the result of one encode even if another is in progress, but they can't render out of order.

@jakearchibald jakearchibald merged commit 3035a68 into master Jun 29, 2018
@jakearchibald jakearchibald deleted the options branch October 2, 2018 14:18
alisaitbilgi pushed a commit to alisaitbilgi/squoosh that referenced this pull request Feb 19, 2019
* Initial work to add Options config

* Use a single encoder instance and retry up to 10 times on failure.

* Switch both sides to allow encoding from the source image, add options configuration for each.

* Styling for options (and a few tweaks for the app)

* Dep updates.

* Remove commented out code.

* Fix Encoder typing

* Fix lint issues

* Apparently I didnt have tslint autofix enabled on the chromebook

* Attempt to fix layout/panning issues

* Fix missing custom element import!

* Fix variable naming, remove dynamic encoder names, remove retry, allow encoders to return ImageData.

* Refactor state management to use an Array of objects and immutable updates instead of relying on explicit update notifications.

* Add Identity encoder, which is a passthrough encoder that handles the "original" view.

* Drop comlink-loader into the project and add ".worker" to the jpeg encoder filename so it runs in a worker (:unicorn:)

* lint fixes.

* cleanup

* smaller PR feedback fixes

* rename "jpeg" codec to "MozJpeg"

* Formatting fixes for Options

* Colocate codecs and their options UIs in src/codecs, and standardize the namings

* Handle canvas errors

* Throw if quality is undefined, add default quality

* add note about temp styles

* add note about temp styles [2]

* Renaming updateOption

* Clarify option input bindings

* Move updateCanvas() to util and rename to drawBitmapToCanvas

* use generics to pass through encoder options

* Remove unused dependencies

* fix options type

* const

* Use `Array.prototype.some()` for image loading check

* Display encoding errors in the UI.

* I fought typescript and I think I won

* This doesn't need to be optional

* Quality isn't optional

* Simplifying comlink casting

* Splitting counters into loading and displaying

* Still loading if the loading counter isn't equal.
alisaitbilgi added a commit to alisaitbilgi/squoosh that referenced this pull request Feb 25, 2019
# This is the 1st commit message:
Add position reset button and update zoom interaction. (GoogleChromeLabs#292) (GoogleChromeLabs#345)

# The commit message GoogleChromeLabs#2 will be skipped:

#	1.2.3

# The commit message GoogleChromeLabs#3 will be skipped:

#	Prevent both sides sharing a download URL. (GoogleChromeLabs#369)
#

# The commit message GoogleChromeLabs#4 will be skipped:

#	Add basic history handling (GoogleChromeLabs#288) (GoogleChromeLabs#309)
#
#	* Add basic history handling (GoogleChromeLabs#288)
#
#	* Move history management to Compress component
#
#	* Remove unused pathname property from history
#
#	* Rename history listener functions
#
#	* Use history.back instead of history.replace
#
#	* Support going forward in history. Persist last selected file in runtime
#
#	* Add netlify redirects file
#
#	* Use 301 status code for redirect
#
#	* Cleanup _redirects file
#
#	* Use 200 status code for redirects
#
#	* Simplify onPopState function
#
#	* Always redirect to 301 with url rewrite
#
#	* Remove redundant history function
#
#	* Remove file check on render. Call openEditor synchronously
#
#	* Use pushState only if user is on the initial screen. Mount history listener in constructor
#
#	* Simplify openEditor condition
#
#	* Update early return condition
#
#	* Rolling abstractions back into the main component

# The commit message GoogleChromeLabs#5 will be skipped:

#	1.3.0

# The commit message GoogleChromeLabs#6 will be skipped:

#	Add renovate.json

# The commit message GoogleChromeLabs#7 will be skipped:

#	Merge pull request GoogleChromeLabs#373 from GoogleChromeLabs/renovate/configure
#
#	Configure Renovate

# The commit message GoogleChromeLabs#8 will be skipped:

#	Pin dependencies

# The commit message GoogleChromeLabs#9 will be skipped:

#	Merge pull request GoogleChromeLabs#374 from GoogleChromeLabs/renovate/pin-dependencies
#
#	Pin dependencies

# The commit message GoogleChromeLabs#10 will be skipped:

#	Update dependency mini-css-extract-plugin to v0.5.0

# The commit message GoogleChromeLabs#11 will be skipped:

#	Merge pull request GoogleChromeLabs#380 from GoogleChromeLabs/renovate/mini-css-extract-plugin-0.x
#
#	Update dependency mini-css-extract-plugin to v0.5.0

# The commit message GoogleChromeLabs#12 will be skipped:

#	Update dependency @types/node to v10.12.14

# The commit message GoogleChromeLabs#13 will be skipped:

#	Merge pull request GoogleChromeLabs#376 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.14

# The commit message GoogleChromeLabs#14 will be skipped:

#	Update dependency @types/node to v10.12.15

# The commit message GoogleChromeLabs#15 will be skipped:

#	Merge pull request GoogleChromeLabs#384 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.15

# The commit message GoogleChromeLabs#16 will be skipped:

#	Update dependency comlink to v3.1.1

# The commit message GoogleChromeLabs#17 will be skipped:

#	Merge pull request GoogleChromeLabs#377 from GoogleChromeLabs/renovate/comlink-3.x
#
#	Update dependency comlink to v3.1.1

# The commit message GoogleChromeLabs#18 will be skipped:

#	Update dependency @types/webassembly-js-api to v0.0.2

# The commit message GoogleChromeLabs#19 will be skipped:

#	Merge pull request GoogleChromeLabs#375 from GoogleChromeLabs/renovate/webassembly-js-api-0.x
#
#	Update dependency @types/webassembly-js-api to v0.0.2

# The commit message GoogleChromeLabs#20 will be skipped:

#	Update dependency critters-webpack-plugin to v2.1.1

# The commit message GoogleChromeLabs#21 will be skipped:

#	Merge pull request GoogleChromeLabs#378 from GoogleChromeLabs/renovate/critters-webpack-plugin-2.x
#
#	Update dependency critters-webpack-plugin to v2.1.1

# The commit message GoogleChromeLabs#22 will be skipped:

#	Update dependency husky to v1.2.1

# The commit message GoogleChromeLabs#23 will be skipped:

#	Merge pull request GoogleChromeLabs#379 from GoogleChromeLabs/renovate/husky-1.x
#
#	Update dependency husky to v1.2.1

# The commit message GoogleChromeLabs#24 will be skipped:

#	Update dependency preact to v8.4.2

# The commit message GoogleChromeLabs#25 will be skipped:

#	Merge pull request GoogleChromeLabs#381 from GoogleChromeLabs/renovate/preact-8.x
#
#	Update dependency preact to v8.4.2

# The commit message GoogleChromeLabs#26 will be skipped:

#	Update dependency ts-loader to v5.3.1

# The commit message GoogleChromeLabs#27 will be skipped:

#	Merge pull request GoogleChromeLabs#382 from GoogleChromeLabs/renovate/ts-loader-5.x
#
#	Update dependency ts-loader to v5.3.1

# The commit message GoogleChromeLabs#28 will be skipped:

#	Update dependency tslint-config-airbnb to v5.11.1

# The commit message GoogleChromeLabs#29 will be skipped:

#	Merge pull request GoogleChromeLabs#383 from GoogleChromeLabs/renovate/tslint-config-airbnb-5.x
#
#	Update dependency tslint-config-airbnb to v5.11.1

# The commit message GoogleChromeLabs#30 will be skipped:

#	Update dependency webpack to v4.27.1

# The commit message GoogleChromeLabs#31 will be skipped:

#	Merge pull request GoogleChromeLabs#386 from GoogleChromeLabs/renovate/webpack-4.x
#
#	Update dependency webpack to v4.27.1

# The commit message GoogleChromeLabs#32 will be skipped:

#	Update dependency raw-loader to v1

# The commit message GoogleChromeLabs#33 will be skipped:

#	Merge pull request GoogleChromeLabs#388 from GoogleChromeLabs/renovate/raw-loader-1.x
#
#	Update dependency raw-loader to v1

# The commit message GoogleChromeLabs#34 will be skipped:

#	Update dependency worker-plugin to v3

# The commit message GoogleChromeLabs#35 will be skipped:

#	Merge pull request GoogleChromeLabs#390 from GoogleChromeLabs/renovate/worker-plugin-3.x
#
#	Update dependency worker-plugin to v3

# The commit message GoogleChromeLabs#36 will be skipped:

#	Fixing sharp & preprocess settings

# The commit message GoogleChromeLabs#37 will be skipped:

#	Using use_argb conditionally

# The commit message GoogleChromeLabs#38 will be skipped:

#	Merge pull request GoogleChromeLabs#393 from GoogleChromeLabs/webp-sharp-fix
#
#	Fixing sharp & preprocess settings. Fixes GoogleChromeLabs#392.

# The commit message GoogleChromeLabs#39 will be skipped:

#	Debouncing input. Fixes GoogleChromeLabs#277 (GoogleChromeLabs#394)
#
#	* Debouncing input
#
#	* Clarifying comment
#
#	* More comments and clarifications

# The commit message GoogleChromeLabs#40 will be skipped:

#	Update dependency typescript to v3.2.2

# The commit message GoogleChromeLabs#41 will be skipped:

#	Fix typings for TypeScript v3.2

# The commit message GoogleChromeLabs#42 will be skipped:

#	Merge pull request GoogleChromeLabs#385 from GoogleChromeLabs/renovate/typescript-3.x
#
#	Update dependency typescript to v3.2.2

# The commit message GoogleChromeLabs#43 will be skipped:

#	Preventing zoom in iOS Safari. (GoogleChromeLabs#395)
#

# The commit message GoogleChromeLabs#44 will be skipped:

#	Update README.md
#
#	closes GoogleChromeLabs#367
#	updating incorrect URL

# The commit message GoogleChromeLabs#45 will be skipped:

#	Merge pull request GoogleChromeLabs#396 from GoogleChromeLabs/kosamari-patch-2
#
#	Update README.md for OptiPNG

# The commit message GoogleChromeLabs#46 will be skipped:

#	1.3.1

# The commit message GoogleChromeLabs#47 will be skipped:

#	Update dependency tslint to v5.12.0

# The commit message GoogleChromeLabs#48 will be skipped:

#	Merge pull request GoogleChromeLabs#398 from GoogleChromeLabs/renovate/tslint-5.x
#
#	Update dependency tslint to v5.12.0

# The commit message GoogleChromeLabs#49 will be skipped:

#	Update dependency husky to v1.3.0

# The commit message GoogleChromeLabs#50 will be skipped:

#	Merge pull request GoogleChromeLabs#399 from GoogleChromeLabs/renovate/husky-1.x
#
#	Update dependency husky to v1.3.0

# The commit message GoogleChromeLabs#51 will be skipped:

#	Update dependency @types/node to v10.12.17

# The commit message GoogleChromeLabs#52 will be skipped:

#	Merge pull request GoogleChromeLabs#400 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.17

# The commit message GoogleChromeLabs#53 will be skipped:

#	Update dependency webpack to v4.28.0

# The commit message GoogleChromeLabs#54 will be skipped:

#	Merge pull request GoogleChromeLabs#402 from GoogleChromeLabs/renovate/webpack-4.x
#
#	Update dependency webpack to v4.28.0

# The commit message GoogleChromeLabs#55 will be skipped:

#	Update dependency @types/node to v10.12.18

# The commit message GoogleChromeLabs#56 will be skipped:

#	Merge pull request GoogleChromeLabs#403 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.18

# The commit message GoogleChromeLabs#57 will be skipped:

#	Update dependency file-loader to v3

# The commit message GoogleChromeLabs#58 will be skipped:

#	Merge pull request GoogleChromeLabs#404 from GoogleChromeLabs/renovate/file-loader-3.x
#
#	Update dependency file-loader to v3

# The commit message GoogleChromeLabs#59 will be skipped:

#	Update dependency ts-loader to v5.3.2

# The commit message GoogleChromeLabs#60 will be skipped:

#	Merge pull request GoogleChromeLabs#406 from GoogleChromeLabs/renovate/ts-loader-5.x
#
#	Update dependency ts-loader to v5.3.2

# The commit message GoogleChromeLabs#61 will be skipped:

#	Update dependency webpack-dev-server to v3.1.11

# The commit message GoogleChromeLabs#62 will be skipped:

#	Merge pull request GoogleChromeLabs#407 from GoogleChromeLabs/renovate/webpack-dev-server-3.x
#
#	Update dependency webpack-dev-server to v3.1.11

# The commit message GoogleChromeLabs#63 will be skipped:

#	Update dependency webpack-dev-server to v3.1.12

# The commit message GoogleChromeLabs#64 will be skipped:

#	Merge pull request GoogleChromeLabs#408 from GoogleChromeLabs/renovate/webpack-dev-server-3.x
#
#	Update dependency webpack-dev-server to v3.1.12

# The commit message GoogleChromeLabs#65 will be skipped:

#	Update dependency terser-webpack-plugin to v1.2.0

# The commit message GoogleChromeLabs#66 will be skipped:

#	Merge pull request GoogleChromeLabs#409 from GoogleChromeLabs/renovate/terser-webpack-plugin-1.x
#
#	Update dependency terser-webpack-plugin to v1.2.0

# The commit message GoogleChromeLabs#67 will be skipped:

#	Update dependency webpack-dev-server to v3.1.13

# The commit message GoogleChromeLabs#68 will be skipped:

#	Merge pull request GoogleChromeLabs#410 from GoogleChromeLabs/renovate/webpack-dev-server-3.x
#
#	Update dependency webpack-dev-server to v3.1.13

# The commit message GoogleChromeLabs#69 will be skipped:

#	Update dependency webpack-dev-server to v3.1.14

# The commit message GoogleChromeLabs#70 will be skipped:

#	Merge pull request GoogleChromeLabs#411 from GoogleChromeLabs/renovate/webpack-dev-server-3.x
#
#	Update dependency webpack-dev-server to v3.1.14

# The commit message GoogleChromeLabs#71 will be skipped:

#	Update dependency loader-utils to v1.2.0

# The commit message GoogleChromeLabs#72 will be skipped:

#	Merge pull request GoogleChromeLabs#412 from GoogleChromeLabs/renovate/loader-utils-1.x
#
#	Update dependency loader-utils to v1.2.0

# The commit message GoogleChromeLabs#73 will be skipped:

#	Update dependency terser-webpack-plugin to v1.2.1

# The commit message GoogleChromeLabs#74 will be skipped:

#	Merge pull request GoogleChromeLabs#414 from GoogleChromeLabs/renovate/terser-webpack-plugin-1.x
#
#	Update dependency terser-webpack-plugin to v1.2.1

# The commit message GoogleChromeLabs#75 will be skipped:

#	Update dependency husky to v1.3.1

# The commit message GoogleChromeLabs#76 will be skipped:

#	Merge pull request GoogleChromeLabs#418 from GoogleChromeLabs/renovate/husky-1.x
#
#	Update dependency husky to v1.3.1

# The commit message GoogleChromeLabs#77 will be skipped:

#	Update dependency webpack-cli to v3.2.0

# The commit message GoogleChromeLabs#78 will be skipped:

#	Merge pull request GoogleChromeLabs#421 from GoogleChromeLabs/renovate/webpack-cli-3.x
#
#	Update dependency webpack-cli to v3.2.0

# The commit message GoogleChromeLabs#79 will be skipped:

#	Update dependency @webpack-cli/serve to v0.1.3

# The commit message GoogleChromeLabs#80 will be skipped:

#	Merge pull request GoogleChromeLabs#420 from GoogleChromeLabs/renovate/webpack-cli-serve-0.x
#
#	Update dependency @webpack-cli/serve to v0.1.3

# The commit message GoogleChromeLabs#81 will be skipped:

#	Update dependency critters-webpack-plugin to v2.1.2

# The commit message GoogleChromeLabs#82 will be skipped:

#	Merge pull request GoogleChromeLabs#415 from GoogleChromeLabs/renovate/critters-webpack-plugin-2.x
#
#	Update dependency critters-webpack-plugin to v2.1.2

# The commit message GoogleChromeLabs#83 will be skipped:

#	Update dependency ts-loader to v5.3.3

# The commit message GoogleChromeLabs#84 will be skipped:

#	Merge pull request GoogleChromeLabs#423 from GoogleChromeLabs/renovate/ts-loader-5.x
#
#	Update dependency ts-loader to v5.3.3

# The commit message GoogleChromeLabs#85 will be skipped:

#	Update dependency critters-webpack-plugin to v2.1.3

# The commit message GoogleChromeLabs#86 will be skipped:

#	Merge pull request GoogleChromeLabs#422 from GoogleChromeLabs/renovate/critters-webpack-plugin-2.x
#
#	Update dependency critters-webpack-plugin to v2.1.3

# The commit message GoogleChromeLabs#87 will be skipped:

#	Update dependency webpack-cli to v3.2.1

# The commit message GoogleChromeLabs#88 will be skipped:

#	Merge pull request GoogleChromeLabs#424 from GoogleChromeLabs/renovate/webpack-cli-3.x
#
#	Update dependency webpack-cli to v3.2.1

# The commit message GoogleChromeLabs#89 will be skipped:

#	Update dependency tslint to v5.12.1

# The commit message GoogleChromeLabs#90 will be skipped:

#	Merge pull request GoogleChromeLabs#427 from GoogleChromeLabs/renovate/tslint-5.x
#
#	Update dependency tslint to v5.12.1

# The commit message GoogleChromeLabs#91 will be skipped:

#	Update dependency typescript to v3.2.4

# The commit message GoogleChromeLabs#92 will be skipped:

#	Merge pull request GoogleChromeLabs#431 from GoogleChromeLabs/renovate/typescript-3.x
#
#	Update dependency typescript to v3.2.4

# The commit message GoogleChromeLabs#93 will be skipped:

#	Update dependency critters-webpack-plugin to v2.2.0

# The commit message GoogleChromeLabs#94 will be skipped:

#	Merge pull request GoogleChromeLabs#432 from GoogleChromeLabs/renovate/critters-webpack-plugin-2.x
#
#	Update dependency critters-webpack-plugin to v2.2.0

# The commit message GoogleChromeLabs#95 will be skipped:

#	Update dependency clean-webpack-plugin to v1.0.1

# The commit message GoogleChromeLabs#96 will be skipped:

#	Merge pull request GoogleChromeLabs#434 from GoogleChromeLabs/renovate/clean-webpack-plugin-1.x
#
#	Update dependency clean-webpack-plugin to v1.0.1

# The commit message GoogleChromeLabs#97 will be skipped:

#	Update dependency progress-bar-webpack-plugin to v1.12.0

# The commit message GoogleChromeLabs#98 will be skipped:

#	Merge pull request GoogleChromeLabs#435 from GoogleChromeLabs/renovate/progress-bar-webpack-plugin-1.x
#
#	Update dependency progress-bar-webpack-plugin to v1.12.0

# The commit message GoogleChromeLabs#99 will be skipped:

#	Add rotate user timing

# The commit message GoogleChromeLabs#100 will be skipped:

#	Use Uint32Array to copy an entire pixel per op

# The commit message GoogleChromeLabs#101 will be skipped:

#	Revert "Add rotate user timing"
#
#	This reverts commit 887db67.

# The commit message GoogleChromeLabs#102 will be skipped:

#	Remove unused bpp

# The commit message GoogleChromeLabs#103 will be skipped:

#	Merge pull request GoogleChromeLabs#436 from GoogleChromeLabs/optimize-rotate
#
#	Optimize rotate

# The commit message GoogleChromeLabs#104 will be skipped:

#	Update dependency @types/node to v10.12.19

# The commit message GoogleChromeLabs#105 will be skipped:

#	Merge pull request GoogleChromeLabs#441 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.19

# The commit message GoogleChromeLabs#106 will be skipped:

#	Update dependency progress-bar-webpack-plugin to v1.12.1

# The commit message GoogleChromeLabs#107 will be skipped:

#	Merge pull request GoogleChromeLabs#442 from GoogleChromeLabs/renovate/progress-bar-webpack-plugin-1.x
#
#	Update dependency progress-bar-webpack-plugin to v1.12.1

# The commit message GoogleChromeLabs#108 will be skipped:

#	Update dependency @types/node to v10.12.20

# The commit message GoogleChromeLabs#109 will be skipped:

#	Merge pull request GoogleChromeLabs#443 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.20

# The commit message GoogleChromeLabs#110 will be skipped:

#	Update dependency @types/node to v10.12.21

# The commit message GoogleChromeLabs#111 will be skipped:

#	Merge pull request GoogleChromeLabs#445 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.21

# The commit message GoogleChromeLabs#112 will be skipped:

#	This fixes GoogleChromeLabs#446 and sometimes it's best not to ask too many questions. (GoogleChromeLabs#447)
#

# The commit message GoogleChromeLabs#113 will be skipped:

#	Update dependency terser-webpack-plugin to v1.2.2

# The commit message GoogleChromeLabs#114 will be skipped:

#	Merge pull request GoogleChromeLabs#448 from GoogleChromeLabs/renovate/terser-webpack-plugin-1.x
#
#	Update dependency terser-webpack-plugin to v1.2.2

# The commit message GoogleChromeLabs#115 will be skipped:

#	# This is a combination of 20 commits.
#	# This is the 1st commit message:
#	Merge from upstream/master branch
#
#	# This is the commit message GoogleChromeLabs#2:
#
#	Update zoom and positioning behaviours
#
#	# This is the commit message GoogleChromeLabs#3:
#
#	Merge branch 'master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#4:
#
#	Merge branch 'master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#5:
#
#	Merge branch 'master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#6:
#
#	Merge branch 'master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#7:
#
#	Merge branch 'master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#8:
#
#	Merge branch 'master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#9:
#
#	Merge branch 'master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#10:
#
#	Merge branch 'master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#11:
#
#	Merge from remote branch
#
#	# This is the commit message GoogleChromeLabs#12:
#
#	Merge from upstream/master
#
#	# This is the commit message GoogleChromeLabs#13:
#
#	Lazy-load the intersection-observer polyfill and optionally control wheel event
#
#	# This is the commit message GoogleChromeLabs#14:
#
#	Fix threshold handling issue
#
#	# This is the commit message GoogleChromeLabs#15:
#
#	merge remote-tracking branch 'upstream/master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#16:
#
#	merge remote-tracking branch 'upstream/master' into update-zoom-interaction
#
#	# This is the commit message GoogleChromeLabs#17:
#
#	Update double click listener and position reset behaviour
#
#	# This is the commit message GoogleChromeLabs#18:
#
#	Change back the indentations
#
#	# This is the commit message GoogleChromeLabs#19:
#
#	Update double click listener and position reset behaviour
#
#	# This is the commit message GoogleChromeLabs#20:
#
#	Change back the indentations

# The commit message GoogleChromeLabs#116 will be skipped:

#	Merge from upstream/master branch

# The commit message GoogleChromeLabs#117 will be skipped:

#	Update zoom and positioning behaviours

# The commit message GoogleChromeLabs#118 will be skipped:

#	Merge branch 'master' into update-zoom-interaction

# The commit message GoogleChromeLabs#119 will be skipped:

#	Merge branch 'master' into update-zoom-interaction

# The commit message GoogleChromeLabs#120 will be skipped:

#	Merge branch 'master' into update-zoom-interaction

# The commit message GoogleChromeLabs#121 will be skipped:

#	Merge branch 'master' into update-zoom-interaction

# The commit message GoogleChromeLabs#122 will be skipped:

#	Merge branch 'master' into update-zoom-interaction

# The commit message GoogleChromeLabs#123 will be skipped:

#	Merge branch 'master' into update-zoom-interaction

# The commit message GoogleChromeLabs#124 will be skipped:

#	Merge branch 'master' into update-zoom-interaction

# The commit message GoogleChromeLabs#125 will be skipped:

#	Merge branch 'master' into update-zoom-interaction

# The commit message GoogleChromeLabs#126 will be skipped:

#	Merge from remote branch

# The commit message GoogleChromeLabs#127 will be skipped:

#	Update dependency webpack-cli to v3.2.3

# The commit message GoogleChromeLabs#128 will be skipped:

#	Merge pull request GoogleChromeLabs#449 from GoogleChromeLabs/renovate/webpack-cli-3.x
#
#	Update dependency webpack-cli to v3.2.3

# The commit message GoogleChromeLabs#129 will be skipped:

#	Update libwebp to 1.0.2 (GoogleChromeLabs#439)
#
#	* Update package.json
#
#	* Update package.json
#
#	* Update README.md
#
#	* Update README.md
#
#	* Use cmake for libwebp
#
#	* Minimize libwebp

# The commit message GoogleChromeLabs#130 will be skipped:

#	Update dependency chokidar to v2.1.0

# The commit message GoogleChromeLabs#131 will be skipped:

#	Merge pull request GoogleChromeLabs#451 from GoogleChromeLabs/renovate/chokidar-2.x
#
#	Update dependency chokidar to v2.1.0

# The commit message GoogleChromeLabs#132 will be skipped:

#	Update dependency loader-utils to v1.2.3

# The commit message GoogleChromeLabs#133 will be skipped:

#	Merge pull request GoogleChromeLabs#413 from GoogleChromeLabs/renovate/loader-utils-1.x
#
#	Update dependency loader-utils to v1.2.3

# The commit message GoogleChromeLabs#134 will be skipped:

#	Update dependency @types/node to v10.12.23

# The commit message GoogleChromeLabs#135 will be skipped:

#	Merge pull request GoogleChromeLabs#453 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.23

# The commit message GoogleChromeLabs#136 will be skipped:

#	Merge from upstream/master

# The commit message GoogleChromeLabs#137 will be skipped:

#	Lazy-load the intersection-observer polyfill and optionally control wheel event

# The commit message GoogleChromeLabs#138 will be skipped:

#	Fix threshold handling issue

# The commit message GoogleChromeLabs#139 will be skipped:

#	Adding CI step to compare build size to previous master build. (GoogleChromeLabs#450)
#

# The commit message GoogleChromeLabs#140 will be skipped:

#	no one must know I did this, or that it got through review.

# The commit message GoogleChromeLabs#141 will be skipped:

#	Pin dependencies (GoogleChromeLabs#456)
#

# The commit message GoogleChromeLabs#142 will be skipped:

#	Switching to 1.4x rather than 140%

# The commit message GoogleChromeLabs#143 will be skipped:

#	Rotate implementation in Rust

# The commit message GoogleChromeLabs#144 will be skipped:

#	Reuse rotate instance and calculate pages correctly

# The commit message GoogleChromeLabs#145 will be skipped:

#	Update wasm build

# The commit message GoogleChromeLabs#146 will be skipped:

#	Merge pull request GoogleChromeLabs#438 from GoogleChromeLabs/rust-rotate
#
#	Rotate implementation in Rust

# The commit message GoogleChromeLabs#147 will be skipped:

#	Fix buffer size/offset calculations in rotate/processor.ts

# The commit message GoogleChromeLabs#148 will be skipped:

#	Merge pull request GoogleChromeLabs#458 from jviide/rust-rotate
#
#	Fix buffer offset/size calculations in rotate/processor.ts

# The commit message GoogleChromeLabs#149 will be skipped:

#	1.3.2

# The commit message GoogleChromeLabs#150 will be skipped:

#	Update dependency webpack-bundle-analyzer to v3.0.4

# The commit message GoogleChromeLabs#151 will be skipped:

#	Merge pull request GoogleChromeLabs#459 from GoogleChromeLabs/renovate/webpack-bundle-analyzer-3.x
#
#	Update dependency webpack-bundle-analyzer to v3.0.4

# The commit message GoogleChromeLabs#152 will be skipped:

#	Update dependency @types/node to v10.12.25

# The commit message GoogleChromeLabs#153 will be skipped:

#	Merge pull request GoogleChromeLabs#455 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.25

# The commit message GoogleChromeLabs#154 will be skipped:

#	Update dependency chokidar to v2.1.1

# The commit message GoogleChromeLabs#155 will be skipped:

#	Merge pull request GoogleChromeLabs#457 from GoogleChromeLabs/renovate/chokidar-2.x
#
#	Update dependency chokidar to v2.1.1

# The commit message GoogleChromeLabs#156 will be skipped:

#	Update dependency @types/node to v10.12.26

# The commit message GoogleChromeLabs#157 will be skipped:

#	Merge pull request GoogleChromeLabs#461 from GoogleChromeLabs/renovate/node-10.x
#
#	Update dependency @types/node to v10.12.26

# The commit message GoogleChromeLabs#158 will be skipped:

#	Updating package lock to fix Netlify

# The commit message GoogleChromeLabs#159 will be skipped:

#	Make Rust rotate code smaller (GoogleChromeLabs#462)
#
#	* Make Rust rotate code smaller
#
#	* Back on the rust happy path

# The commit message GoogleChromeLabs#160 will be skipped:

#	1.3.3

# The commit message GoogleChromeLabs#161 will be skipped:

#	Update dependency chokidar to v2.1.2

# The commit message GoogleChromeLabs#162 will be skipped:

#	Merge pull request GoogleChromeLabs#467 from GoogleChromeLabs/renovate/chokidar-2.x
#
#	Update dependency chokidar to v2.1.2

# The commit message GoogleChromeLabs#163 will be skipped:

#	merge remote-tracking branch 'upstream/master' into update-zoom-interaction

# The commit message GoogleChromeLabs#164 will be skipped:

#	Update double click listener and position reset behaviour

# The commit message GoogleChromeLabs#165 will be skipped:

#	Change back the indentations

# The commit message GoogleChromeLabs#166 will be skipped:

#	Update double click listener and position reset behaviour

# The commit message GoogleChromeLabs#167 will be skipped:

#	Change back the indentations
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

Successfully merging this pull request may close these issues.

None yet

4 participants