Skip to content

Conversation

mstriemer
Copy link
Contributor

I didn't like the other one so I started from scratch and used it as a guide. I think it's looking a bit cleaner.

disco-oediv mov

Fixes mozilla/addons#9582.
Supersedes #430.

@mstriemer mstriemer mentioned this pull request May 24, 2016
@mstriemer
Copy link
Contributor Author

I updated the sizing on the video to 512x288 since that's half of the video's 1024x576. The border radius wasn't working with the other dimensions since there'd be whitespace on the sides so you couldn't see it.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling c514e48 on mstriemer:oediv-314 into 9ac1063 on mozilla:master.

test: /\.jpg$/,
loader: 'url?limit=10000&mimetype=image/jpeg',
}, {
test: /\.webm$/,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for the videos you could use the file-loader directly since these should never be inlined. The url-loader hands-off to the file-loader if the size is over the limit param.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we had a small video wouldn't we want it inlined though? I feel like the benefits would be the same as for images. I doubt we'll have a video that small but none the less.

@muffinresearch
Copy link
Contributor

Lgtm, we'll need to double check this when we have rtl, but it looks great.

r+wc

@mstriemer
Copy link
Contributor Author

@pwalm does the screencast look good?

@pwalm
Copy link
Contributor

pwalm commented May 25, 2016

@mstriemer Holy hell, does it ever. That looks awesome!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 33fbff1 on mstriemer:oediv-314 into 9ac1063 on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f281089 on mstriemer:oediv-314 into 9ac1063 on mozilla:master.

@mstriemer mstriemer merged commit f4cde03 into mozilla:master May 25, 2016
@mstriemer mstriemer deleted the oediv-314 branch May 25, 2016 23:59
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.

Discover video
4 participants