Skip to content

Menu position property behaves in a confusing way when referencing items that occur later in the list #11668

@captbaritone

Description

@captbaritone
  • Electron version: 1.7.8
  • Operating system: Mac OS High Sierra

The position attribute of context menu items passed to buildFromTemplate lets you position items relative to other items. However, it has an undocumented limitation that the referenced item must precede the item that references it.

This can be clearly seen by the fact that indexToInsertByPosition only gets passed the "positioned" elements.

idx = (item.position) ? indexToInsertByPosition(positioned, item.position) : idx += 1

A complete solution would probably involve building a graph and then sorting that graph and handling any potential cycles. The potential for, and handling of, cycles is another fact that the documentation does not touch on.

An API which offers to order items for you, but also requires you to ensure a specific order of input seems less than useful. In fact, it makes me question to what degree is this property being actively used. Additionally, it only sorts items within an ordered list which was explicitly handed to Electron. I don't see a clear use-case where the application itself couldn't simply pre-order the menu items before passing them to buildTemplate.

I believe the property was added as a precursor to adding positioning to Atom. Atom support was never added, and after spending some time looking into it this week, I don't think this API will work for Atom at all.

Possible solutions

  1. Document the shortcomings of this property.
  2. Deprecate this property and discourage its use going forward.
  3. Fix the implementation with a graph sort (I'm not sure how/if the endof could be implemented).

Expected behavior

Menu lists items in order: "One, Two"

Actual behavior

Menu lists items out of order: "Two, One"

screen shot 2018-01-18 at 9 59 16 am

How to reproduce

const {remote} = require('electron')
const {Menu, MenuItem} = remote

const menu = Menu.buildFromTemplate([
    {label: 'Two', id: '2', position: 'after=1'},
    {label: 'One', id: '1'},
])

menu.popup(remote.getCurrentWindow())

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions