Closed Bug 1015769 Opened 10 years ago Closed 10 years ago

Use gaia-menu component in vertical homescreen

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S3 (6june)

People

(Reporter: kgrandon, Assigned: kgrandon)

References

Details

(Whiteboard: [p=2],[systemsfe])

Attachments

(1 file)

Now that we have a simple component, we should use it in the vertical homescreen.
Attached file Github pull request
Hey guys - just looking for some feedback in using this component for the homescreen.

Cristian - I'm not sure how familiar you are with our web component work, but the plan is to replace the old html import elements polyfill with real web components. I think moving our menu/confirm dialogs into a component would make sense. If you like this, and would like to do the confirm dialog, let's file a bug for that as well. (The confirm dialog should mostly work https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_confirm/examples/index.html)

Amir - also some changes landed recently that fixed a platform issue which was causing the menu to render incorrectly. Usage for smart collections may also be fixed now.
Attachment #8428414 - Flags: review?(crdlc)
Attachment #8428414 - Flags: review?(amirn)
Comment on attachment 8428414 [details] [review]
Github pull request

The code looks good to me although I am wondering which animation we are performing with this change. We are only toggling the display property for a menu?
Attachment #8428414 - Flags: review?(crdlc) → review+
Comment on attachment 8428414 [details] [review]
Github pull request

Thanks Cristian! Yes - we do currently lose the transition, but I do think this is ok. Seems like we may want the same transition for confirm as the menu anyway? I'll gladly revisit it this week. Perhaps we can expose some API on the component to easily do this (then we can share code between all apps). E.g., document.getElementById('menu').transition('slide-in')

Amir - let me know if you have any follow-up concerns about this.
Attachment #8428414 - Flags: review?(amirn)
Landed: https://github.com/mozilla-b2g/gaia/commit/68c3018b71e3a70a9f6575b3ab87b945539f4cc5
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to Kevin Grandon :kgrandon from comment #1)
> 
> Amir - also some changes landed recently that fixed a platform issue which
> was causing the menu to render incorrectly. Usage for smart collections may
> also be fixed now.

Tested, indeed fixed! Do you know what CSS was causing the bug?

(In reply to Kevin Grandon :kgrandon from comment #3)
> 
> Amir - let me know if you have any follow-up concerns about this.
I think |show| and |hide| methods can be useful: https://github.com/EverythingMe/gaia/blob/8a291f7e8240ad12be778f93eeeb29ff773aa602/shared/elements/gaia_menu/script.js#L29-L35
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: