Closed Bug 1816567 Opened 2 years ago Closed 2 years ago

Create a top-level `DropdownMenu` Composable

Categories

(Fenix :: General, task, P3)

All
Android
task

Tracking

(firefox113 fixed)

RESOLVED FIXED
113 Branch
Tracking Status
firefox113 --- fixed

People

(Reporter: 007, Assigned: aputanu)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fxdroid])

Attachments

(1 file)

https://www.figma.com/file/ujdiReypSypu2GhRgzArea/Android-Components?node-id=1808%3A7913&t=lbpgP5pE8qXx6YpY-0

  • Support Light/Dark/Private theming
  • Support text only items and text + icon items this will be covered in another ticket

A nice-to-have would be for the API to allow for a list of menu items (whatever form that takes) to make it so it's just one function call to set up the:

  • Menu
  • Menu visibility
  • Menu items
  • Click callbacks for each item

As part of this ticket, please replace the existing usages of androidx.compose.material.DropdownMenu with our new DropdownMenu Composable. This [searchfox link](https://searchfox.org/mozilla-mobile/search?q=DropdownMenu(&path=) should contain them all (ignoring the references to the usages in Focus)

RecentTabMenu and RecentTabMenuItem could be some good jumping off points.

Severity: -- → N/A
Assignee: nobody → aputanu

We already have quite a few DropdownMenu composable that are more or less duplicate of eachother. So, I imagine the point of this is to unify all of this in a common Menu composable. https://searchfox.org/mozilla-mobile/search?q=DropdownMenu(&path= Am I understanding the task correctly?

Just further thoughts here, I would prefer if we went about this incrementally and not add all the APIs and functionality that we aren't actively using. Step one would be to unify all of these DropdownMenu composables while supporting Light/Dark/Private theme - I think we should accomplish that only with this ticket.

We don't necessarily need to support text + icons immediately and that could be made into a followup of this bug since most of the DropdownMenu examples linked don't have icons in them and I don't foresee an immediate need to replace our browser-menu. I would like to just see us go a bit more slowly to come up with a solid foundation before we continue to evolve the API to further our needs.

Flags: needinfo?(nbond)
Flags: needinfo?(aputanu)

So, I imagine the point of this is to unify all of this in a common Menu composable. https://searchfox.org/mozilla-mobile/search?q=DropdownMenu(&path= Am I understanding the task correctly?

Yes, aside from the Focus use cases that the link shows, that is the intent here.

Just further thoughts here, I would prefer if we went about this incrementally and not add all the APIs and functionality that we aren't actively using. Step one would be to unify all of these DropdownMenu composables while supporting Light/Dark/Private theme - I think we should accomplish that only with this ticket.

I think this makes sense. The only location I'm currently aware of that uses the Menu with icons is the three dot menu in the awesome bar. There has yet to be any talk about converting that component to Compose, so the work to create a Menu with a mix of text and text + icon items could certainly be deferred.

I will update the body of this ticket and add a note to the Compose library meta ticket that the text + icon Menu items need a ticket filed when appropriate.

Flags: needinfo?(nbond)
Summary: Create a top-level Menu Composable → Create a top-level `DropdownMenu` Composable

In the Figma designs I see that we also intend to have the Menu items displayed horizontally, I believe this is to replace the ContextMenu. I'm not sure if this is supported by the Compose library DropdownMenu. Should we also consider filing a ticket for this when needed, or should we add the support for it in this ticket?

Flags: needinfo?(aputanu)

We should file it when needed. I admittedly had not even seen that part of the figma, and, yeah, I'm not sure either if the Compose library DropdownMenu has the API for that.

I'll make a note in the Compose meta

Attached file GitHub Pull Request
Status: NEW → RESOLVED
Closed: 2 years ago
Flags: qe-verify+
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: