Can you edit the description to explain where to get js/lib/gif.js ?

Col: 9 Do not use 'new' for side effects.

Col: 60 Missing semicolon.

Col: 46 Missing semicolon.

Col: 5 'GifEncoderAbstraction' was used before it was defined.

Why is this change necessary, out of curiosity? It looks unrelated to your project.

What kind of configurability are you envisioning here? The "For now" comment doesn't give us much to work with - …

What is the strange bug in Chrome?

Please remove this newline.

Please remove this newline.

Can you please document each of these functions?

Please remove this newline.

This is how we generally do this sort of thing: https://github.com/reviewboard/reviewboard/blob/b637ecfcfff04fbe493fb6186e7be4af5745503e/reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js#L10-L54 Note that it's important to keep the indentation for …

I noticed that you're putting the strings for other things inside GIF_COMMENT_UI_STRINGS. Why not these as well?

You don't need to set a value to the disabled attribute.

Can you invert this condition, so that we return if $mainContainer.length > 0, and then otherwise continue on below?

Space after //

Please remove this newline.

Please remove this newline, and the one at the end of the function.

Col: 9 Do not use 'new' for side effects.

Please remove this newline.

Space after //

Trailing whitespace

Space after //

Let's format this: navigator.mediaDevices.getUserMedia({ video: VIDEO_CONFIG, audio: false }).then(this.displayMediaStream.bind(this)) .catch(this.displayMediaStreamError.bind(this));

Please remove this extra newline.

Please remove this dead code.

These extra newlines can go.

Col: 5 'GifEncoderAbstraction' was used before it was defined.

Extra newline can be removed.

Extra newline can be removed.

Maybe mention that they can be attached to Review Board comments, so: Record GIFs from your webcam and attach them …

Col: 27 Expected '!==' and instead saw '!='.

E501 line too long (89 > 79 characters)

Please add a docstring.

Please add a docstring.

Please add a docstring.

Add another blank line here.

This only works with the default static media path. This should instead be: (at the top) from django.contrib.staticfiles.templatetags.staticfiles import static …

Add a trailing comma here.

This should probably also be const instead of var

Doc comments like this should be in a very similar format to python docstrings. Only major difference is in the …

Since this is ES6 we can use member function sugar: injectUiIfNotInDom() { } This should probably also have "UI" and …

This can make use of the dedent JS template string processor: ``javascript const mainContainerHtml = dedent <div class="gif-container"> <div style="max-height: …

Instead of doing function() {}.bind(this), you can use a fat arrow function, which have lexical scoping for this: $saveButton.click(ev => …

This can be const.

This blank line can go away.

This blank line can go away.

Please fix up formatting of this docstring to match our doc standards.

Use ES6 member function sugar.

Please fix up formatting of this docstring to match our doc standards.

Use ES6 member function sugar.

Please fix docstring syntax.

Use ES6 member function sugar.

Instead of using inline style here, please just put it in the LESS file for the .gifcomment-control-open class. Also, please …

This is a good place to use a fat arrow function.

Join these two lines together.

This is a good place to use a fat arrow function.

This should use jquery accessors: this.$toggleMainContainerButton .prop('disabled', !$enableMarkdownCheckbox.prop('checked'));

Since it's only used once, you can skip assigning this to a name. This is also a good place to …

Add another blank line.

Can use ES6 function sugar.

We're moving away from _super because it has some issues. This should just be explicit: RB.Extension.prototype.initialize.call(this)

Add a trailing comma.

Add a trailing comma.

This isn't an ES6 file so it needs to use var. Or you could change it to be .es6.js.

E501 line too long (97 > 79 characters)

E501 line too long (92 > 79 characters)

This was probably copied from other code, but it uses an old format for docstrings. See https://www.notion.so/reviewboard/Writing-Codebase-Documentation-e16312b5f061437cb73cbfa369ac3cb5

Just confirming , if there's a micro or patch, we still bump up the micro version?

Single quotes

Same here.

Same here.

The summary line of doc comments should be in the imperative mood (Create) rather than passive (Creates)

Wrap to 80 columns

Should be in the format of: Returns: jQuery: The newly created or previously existing DOM element... See https://www.notion.so/reviewboard/Writing-Codebase-Documentation-e16312b5f061437cb73cbfa369ac3cb5

Blank line after this.

Blank line after this.

This should be: ``javascript dedent <div class="gif-container"> </div> `;

Should be imperative mood ("Display")

Should be imperative mood ("Close")

Blank line after this.

Blank line after this.

Should be imperative mood ("Inject")

Wrap to 80 columns.

There's an extra space character at the end of this line.

Blank line after this.

Blank line after this.

Blank line after this.

Blank line after this.

Add a doc comment?

Should be imperative mood ("Create")

Should use "Args:" and "Returns:" format as described in our codebase docs guide.

Blank line after this.

Trailing comma.

Adds -> Add

Should use Args/Returns

Renders -> Render

Should use Args/Returns

Checks -> Check Returns: boolean: Whether the current browser has the getUserMedia function.

Displays -> Display

Args:

Initializes -> Initialize

Shutdowns -> Shut down the

Triages and displays -> Triage and display

Wrap to 80 columns. We should also probably use dedent... here.

Should be } else {

Pops up -> Pop up

Let's wrap as: navigator.mediaDevices.getUserMedia({ ... }) .then(...) .catch(...);

Trailing comma.

Renders -> Render

Args:

Can be: this.$previewVideo.onloadedmetadata = () => { this.startRecButton.disabled = false; }

Enables -> Enable

Enables -> Enable These two lines also need to be indented 1 more space.

Disables -> Disable

Updates -> Update

Args:

Starts -> Start

Blank line after this.

Wrap to 80 columns.

Stops -> Stop

Blank line after this.

Use a fat arrow function instead of function(blob) {}.bind(this)

Args:

Wrap to 80 columns

These should be indented 4 spaces (currently they're indented 1).

This can be const-- or even better, just do return new GIF({ ... });

Same here as my previous comment regarding indentation with dedent