Page MenuHomePhabricator

OOjs UI: InlineMenuWidget is not a kind of MenuWidget and should be renamed
Closed, ResolvedPublic

Description

Expected syntax:

var menu = new oo.ui.InlineMenuWidget();
menu.addItems( items );
menu.chooseItem( item1 );
menu.on( 'select', doStuff );

Actual syntax:

var menu = new oo.ui.InlineMenuWidget();
menu.getMenu().addItems( items );
menu.getMenu().chooseItem( item1 );
menu.getMenu().on( 'select', doStuff );

This feels unintuitive (especially with on() which is misleading to debug since InlineMenuWidget has its own event handling but it never fires any events). That InlineMenuWidget calls the functionality of MenuWidget via composition is an implementation detail that should have no effect on the interface.


Version: unspecified
Severity: enhancement

Details

Reference
bz70968

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 3:51 AM
bzimport set Reference to bz70968.
Tgr created this task.Sep 17 2014, 10:24 PM

InlineMenuWidget is just one of the wrappers for MenuWidget (the other is ComboBoxWidget) and not a MenuWidget itself, so it kind of makes sense for it to have a different interface (even if it is surprising at first).

Do you think we should duplicate the public API of MenuWidget on InlineMenuWidget (including re-emitting events)? We'd probably want to do the same for ComboBoxWidget if yes (I think there is value in keeping their interfaces similar). I'm not entirely sure if we should do this, though.

Or perhaps we should just rename InlineMenuWidget to DropdownWidget or something to avoid name confusion?

Renaming seems sane.

Change 170171 had a related patch set uploaded by Bartosz Dziewoński:
[BREAKING CHANGE] Rename InlineMenuWidget → DropdownWidget

https://gerrit.wikimedia.org/r/170171

Change 170175 had a related patch set uploaded by Bartosz Dziewoński:
Update OOjs UI to v0.1.0-pre (d6dbeb1ce6)

https://gerrit.wikimedia.org/r/170175

Change 170179 had a related patch set uploaded by Bartosz Dziewoński:
Change OO.ui.InlineMenuWidget → OO.ui.DropdownWidget for OOUI upgrade

https://gerrit.wikimedia.org/r/170179

Change 170171 merged by jenkins-bot:
[BREAKING CHANGE] Rename InlineMenuWidget → DropdownWidget

https://gerrit.wikimedia.org/r/170171

Change 170175 merged by jenkins-bot:
Follow-up Ifb7ffb1: Update demo.js for breaking OOUI change

https://gerrit.wikimedia.org/r/170175

Change 170179 merged by jenkins-bot:
Change OO.ui.InlineMenuWidget → OO.ui.DropdownWidget for OOUI upgrade

https://gerrit.wikimedia.org/r/170179