Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

removeMenuItem() API #2072

Merged
merged 16 commits into from
Nov 7, 2012
Merged

removeMenuItem() API #2072

merged 16 commits into from
Nov 7, 2012

Conversation

peterflynn
Copy link
Member

Adds a new Menu API for removing existing menu items.

This is just polishing off an existing patch from @tpryan, #1193. The additional changes here are:

  • Merge with master
  • Fix unit tests to use unique command/menu ids since all of Menus-test now shares one window
  • Some code cleanups in unit tests.
  • Move tests to better spot in file.
  • Small code/docs cleanups in removeMenuItem() -- mainly to address this comment

Terrence Ryan and others added 16 commits June 28, 2012 21:23
…nto pflynn/remove-menuitem

* 'removeMenuItem' of https://github.com/tpryan/brackets:
  Renaming variable to make it, you know, refer to what it actually is.
  Testing for proper handling of junk being thrown into the removeMenuItem function.
  Added exception handling for cases where someone throws junk into the function
  Adding test to verify that removing by command or commandID string would work.
  Removing Extraneous DIVIDER code
  Cleaning out some whitespace from my addition to the test.
  Fixed remove menu item to remove the parent <li> of the <a> link that contains it.
  Added ability to accept commands as well as id's.
  Fixing comment grammar mistake.
  Made changes to code in line with @peterflyn feedback here: #1193
  Checking in the addition of RemoveMenuItem.  Also made file pass jslint.

Conflicts:
	test/spec/Menu-test.js
…tests

in spec share one window (use unique ids for everything).
Code cleanup in removeMenuItem(): use existing utility method to compute
menu item id. Plus some docs tweaks.
@peterflynn peterflynn mentioned this pull request Nov 7, 2012
@peterflynn
Copy link
Member Author

I realized after the fact that relocating the unit tests within the file makes it hard to diff my changes vs. the original (to see the part that hasn't been code reviewed already).

But you can always copy-paste the two ranges from:
https://raw.github.com/tpryan/brackets/c13722f1a98de4e3fead35d572f28e1ffa5ea9e3/test/spec/Menu-test.js
https://raw.github.com/adobe/brackets/b76cc1e5d725df128bc6cd8ab16fb45856a8af96/test/spec/Menu-test.js
...and put them in temp files to diff them directly.

@njx
Copy link

njx commented Nov 7, 2012

Looks fine, merging.

njx pushed a commit that referenced this pull request Nov 7, 2012
@njx njx merged commit 442ad06 into master Nov 7, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants