1095674 - [gaia-components] Allow gaia-carousel to be disabled

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Components, defect)

Description

[Justin D'Arcangelo [:justindarc]]

The <gaia-carousel> web component needs to be able to be disabled for interoperability with media_frame.js for use in Gallery.

Comment 1

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Wilson: Please review some minor changes I've made to <gaia-carousel>. The primary thing is the ability to disable the component via `carousel.disabled = true;` in JS or `<gaia-carousel disabled="true"></gaia-carousel>` in HTML. This is necessary in order to allow the component to work with MediaFrame content. This allows us to disable the carousel to prevent the carousel from scrolling when panning a zoomed image until the edge is reached. I've also added a new `willresetitem` event that gets fired when one of the carousel items is moved off-screen. We can use this event to call `.reset()` on the MediaFrame to reset the pan/zoom settings.

Also, I've updated the example app to include a demonstration of using <gaia-carousel> with MediaFrame content. As a result, the PR is rather large due to the inclusion of media_frame.js and all its dependencies. So, when you're reviewing, you can safely ignore everything under example/js/lib/*.

Thanks!

Comment 2

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8519106 [details] [review]
pull-request (master)

Changing review to Diego since Wilson is out this week.

Comment 3

[Diego Marcos [:dmarcos]]

Comment on attachment 8519106 [details] [review]
pull-request (master)

The patch looks good to me. Talked to Justin over IRC and suggested factoring out some parts to make it more readable and maintainable:

- Factor out custom events triggers to something like this.emit('event-name', params)
- Factor out attributes configuration to a common function configureAttribute('attribute-name', value, default, type)

Comment 4

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/gaia-components/gaia-carousel/commit/665de6ebff4b8586ef6480092593d740eae6443d

Comment 5

[Wilson Page [:wilsonpage]]

(In reply to Justin D'Arcangelo [:justindarc] from comment #1)
> Wilson: Please review some minor changes I've made to <gaia-carousel>. The
> primary thing is the ability to disable the component via `carousel.disabled
> = true;` in JS or `<gaia-carousel disabled="true"></gaia-carousel>` in HTML.

I know I'm a bit late, but to be inline with existing HTML `disabled` behaviour, `<gaia-carousel disabled>` should also work.