Page MenuHomePhabricator
Create Task
Maniphest T72968

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

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 Medium.Nov 22 2014, 3:51 AM
• bzimport added a project: OOjs-UI-Technical-Debt.
• bzimport set Reference to bz70968.
Tgr created this task.Sep 17 2014, 10:24 PM
matmarex added a comment.Oct 4 2014, 8:31 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?

Jdforrester-WMF added a comment.Oct 29 2014, 10:53 PM

Renaming seems sane.

gerritbot added a comment.Oct 30 2014, 9:35 PM

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

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

gerritbot added a comment.Oct 30 2014, 9:54 PM

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

gerritbot added a comment.Oct 30 2014, 9:55 PM

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

gerritbot added a comment.Oct 31 2014, 11:00 PM

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

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

gerritbot added a comment.Oct 31 2014, 11:37 PM

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

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

gerritbot added a comment.Nov 1 2014, 1:22 AM

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

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

Jdforrester-WMF edited projects, added OOUI, Technical-Debt; removed OOjs-UI-Technical-Debt.Feb 17 2016, 7:58 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits