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

RESOLVED FIXED in 2.1 S9 (21Nov)

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: justindarc, Assigned: justindarc)

Tracking

unspecified
2.1 S9 (21Nov)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
The <gaia-carousel> web component needs to be able to be disabled for interoperability with media_frame.js for use in Gallery.
(Assignee)

Comment 1

4 years ago
Created attachment 8519106 [details] [review]
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!
Attachment #8519106 - Flags: review?(wilsonpage)
(Assignee)

Comment 2

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

Changing review to Diego since Wilson is out this week.
Attachment #8519106 - Flags: review?(wilsonpage) → review?(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)
Attachment #8519106 - Flags: review?(dmarcos) → review+
(Assignee)

Updated

4 years ago
Assignee: nobody → jdarcangelo
(Assignee)

Comment 4

4 years ago
Landed on master:

https://github.com/gaia-components/gaia-carousel/commit/665de6ebff4b8586ef6480092593d740eae6443d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Target Milestone: --- → 2.1 S9 (21Nov)
(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.
You need to log in before you can comment on or make changes to this bug.